Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DRAFT: Re-implement matching algorithim in NodeJS #1

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Cobular
Copy link

@Cobular Cobular commented Jan 10, 2022

Hey yall! Want to make a PR to track the rest of the progress on this, since I'm going to need some more specific feedback to make much more progress.

Todo / status:

  • Implement core matching algorithm as seen in the Python version
  • Improve on the previous version by finding a more optimal matching through ✨ advanced hill climbing algorithms ✨ (randomly mixing up the assignment order and running it 50 times)
  • Feed data from the database into the algorithm
  • Figure out how auth would work for the query and what I need to do to get myself a token to test
  • Figure out how to convert the match into data usable by the end user
    • Create a new matching result graphql type possibly? May be possible to use existing type, have not explored the types much yet.
  • Create mutation for saving a valid match back to database after human review
  • Figure out how to store a given match into the database

That should be just about everything left to do! I'd like to chat sometime soon with one or both of you about the rest of this, whoever would be the most applicable.

Realized the extraneous work being done in findMatching.ts and removed that
Added more metrics to matchingHelpers.ts
Created more robust data typing in matchingTypes.ts
Created more simple way to test the algorithm's implementation
… it multiple times to find the best solutions.
@tylermenezes
Copy link
Member

tylermenezes commented Jan 10, 2022

Figure out how auth would work for the query

Basically you just want this: https://github.com/codeday/labs-gql/blob/main/src/resolvers/Stats.ts#L17

and what I need to do to get myself a token to test

If you're running it locally you can sign a JWT of this format - https://github.com/codeday/labs-gql/blob/main/src/context/auth/JwtToken.ts - probably just { "typ": "a" } in this case

Once you have it ready I can deploy it and give you a testing token.

Create a new matching result graphql type possibly?

I think that's the best way, even if another type could be repurposed using a dedicated one offers more type safety for clients.

Create mutation for saving a valid match back to database after human review
Figure out how to store a given match into the database

That's already done @ https://github.com/codeday/labs-gql/blob/main/src/resolvers/Project.ts#L71

Once it's possible to download a match-list, I can create a frontend which has a "confirm" button for each possible match.

@Cobular
Copy link
Author

Cobular commented Jan 16, 2022

A few more follow up questions!

  1. Will the database have multiple years / lab sessions worth of projects in it? If so, should I filter on createdAt or updatedAt or something like that?
  2. Any other things I should filter out of a matching? Planing on not processing draft projects but that's all I really see.
  3. Would you prefer if I return a list of students and their matched projects or projects and their matched students? How much data would you like to receive? The project mutation you linked me to could be compatible pretty easily with either approach so it really comes down to what would be more convenient for the planned frontend. To make some progress I'm going to just return it per project as that's more inline with the way data is stored internally but it would be pretty much trivial to transform it later, just let me know!
    Thanks!

…edback and needs additional testing before ready for review.
@tylermenezes
Copy link
Member

  1. Yes but don't worry about it for now. There's another branch with a sessionId which I haven't finished yet, but I can add that in later.
  2. That's it!
  3. You could return a new MatchTuple type so we can display it either way. Might make final matching adjustments easier.

@Cobular
Copy link
Author

Cobular commented Jan 22, 2022

Ok cool! I've created the response with the MatchTuple, it should work properly but I cannot get the auth to load correctly for me on Playground so I can't test it right now. I think it's also kind of dependent on elastic to run, I tried to rip it out the best I could but it wasn't really enough to test properly. The types all check out and I'm pretty confident it should work though! Specifically, I return the matching stats and the MatchTuples so you can get an idea of some properties of the matching if you want as well but that could totally be removed if you'd like. If you could test this on some setup that has everything that'd be great.

Also, I've gone and re-named all the old "Matching" stuff (related to elastic and things) to "Recommendation". I think this is a better name and should reduce confusion between the two types, although editing the GraphQL names of these may cause issues and I'm not sure how to deal with that properly in GraphQL so I may need to put those back. Can you advise on this?

@tylermenezes
Copy link
Member

This looks great to me, I have to change the queries in www-labs before merging however

@Cobular
Copy link
Author

Cobular commented Jan 26, 2022

Sounds good! Please let me know once this is good to be tested, I'd like to validate it on prod before it's ready for prime time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants