Rjf/dispatcher req sort#206
Merged
robfitzgerald merged 3 commits intomainfrom Jun 16, 2023
Merged
Conversation
Collaborator
Author
|
read the docs failed with the following error (below). link to build. it looks like we need to pin our sphinx version to "<7.0.0" as readthedocs doesn't support sphinx 7 yet. |
nreinicke
approved these changes
Jun 16, 2023
Collaborator
nreinicke
left a comment
There was a problem hiding this comment.
Dude! Great find! It would have taken me forever to track that down. So happy you had the lighting bolt moment ⚡
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@nreinicke i think i found it. i was writing an email response to dylan and was jotting down some potential leads, like looking into whether scipy guarantees the linear_sum_assignment is deterministic. then i was like OH YA i should quadruple-check that we build the matrix deterministically. we do not.
in the dispatcher logic, we were creating the assignment matrix of available vehicles by current unassigned requests. the
unassigned_requestscollection was being sorted by value descending, which is to say, "put the highest-valued customers at the top of this list". but there was no tie-breaker:I fixed this by making the sort key
lambda r: (-r.value, r.id). I ran manhattan.yaml twice in a row and observed the top-level stats were the same:as you'll see in the changed files for this PR, i've also updated the newly-added design.md section of the HIVE documentation to clearly highlight the sort issue.