Closed sharkiteuthis closed 5 years ago
this is ready except for tests, but the issue from #115 is blocking
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 Roster::__eq__
to use the set of player ids, so order doesn't matter when using ==
, then added Roster::exact_equal
which respects order and tightened up the existing tests to use exact_equal
where appropriate.
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.
@BenBrostoff once the builds pass this should be good to go
Some tests still failing, just putting this up as a WIP so I can reference it