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

Improve tree placement #60

Merged
merged 6 commits into from
Jul 21, 2023
Merged

Improve tree placement #60

merged 6 commits into from
Jul 21, 2023

Conversation

skieffer
Copy link
Collaborator

This improves the tree placement phase of the HOLA algorithm.

Old behavior: Choose a single best placement, then fail if expansion constraints prove infeasible.

New behavior: Try best placement. If infeasible, backtrack and try next best.

Old behavior: Choose a single best placement, then fail if expansion constraints prove infeasible.

New behavior: Try best placement. If infeasible, backtrack and try next best.
The `listAllPossibleTreePlacements()` methods no longer take any `HolaOpts` into account.
Having `listAllPossibleTreePlacements()` now return ordinal placements, even when cardinal placements are
preferred, results in some necessary adjustments to our tests.

In the case of `nudgeopt.cpp`, the adjustment is due to instability in the sorting methods employed at the start
of the `chooseBestPlacement()` function. `S` placement is now chosen instead of `W`, due to arbitrary ordering.
Seems that certain outcomes may differ, depending on the test runner.
Maybe a consequence of differences in RNG?
@skieffer skieffer merged commit 2e09ffb into master Jul 21, 2023
1 check passed
@skieffer skieffer deleted the tree-placement-crash branch July 21, 2023 15:02
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.

None yet

1 participant