-
Notifications
You must be signed in to change notification settings - Fork 113
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
Feature/lineup constraints #113
Conversation
fix failing test -- order doesn't matter
Some tests still failing, just putting this up as a WIP so I can reference it |
this is ready except for tests, but the issue from #115 is blocking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome, going to use for NBA Christmas games today. I released 2.2.0
if you want to give it a spin - here it is working on repl.it. My change requests are super minor, only important one to me is more detailed error messaging when no lineup solution is found.
I think there's a bug here with multi position players and groups, I'm going to try to reproduce and create a testcase |
FYI (this may be unrelated to the bug you're chasing) but I've noticed inconsistency in test runs with multi position players (maybe ortools related?). For NBA specifically, the same test will produce lineups with the same players but in different positions - I've worked around this just by hackily rewriting tests. |
yeah, it's because of that name to index map in the solver. i need to index on name+position or something |
can you commit a test (comment it out so it doesn't fail, i guess, or just send me the setup) that produces duplicate lineups? |
Yep, it's this one. I think this started intermittently failing when I added the minimum 2 teams rule (which actually only really helps for Showdown, it's min 2 teams and max 4 players on one team for Classic). The roster tested here used to always be 3 guards, 4 forwards and 1 center. Sometimes this passes, but I've noticed occasionally it will be 4/3/1 or 3/3/2. |
It looks like that's an ortools thing, I noticed in the NFL tests, it will sometimes choose different DSTs with the same projections. I changed that test to generate a bunch of rosters and make sure they're all the same. I changed There was definitely a bug for groups with multi position players, so the name->idx map is now a name->set(idx) map, which I think should fix things, but I need to write tests. I should be ready to have this re-reviewed sometime later today. |
- fix DST projections - make nba test slightly faster - seperate roster quality and exact equality
@BenBrostoff once the builds pass this should be good to go |
Player count: {} | ||
''' | ||
.format(optimizer_settings, len(players or [])) | ||
OPTIMIZER CONSTRAINTS: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, I'll do another release once this is merged.
No description provided.