Rjf/label dominates #448
Merged
Rjf/label dominates #448
Conversation
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
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces label dominance checking to the RouteE Compass search algorithm by adding a compare method to the LabelModel trait. The goal is to prune dominated search tree entries during A* search to improve performance by reducing the label state space.
Key changes:
- Added
comparemethod toLabelModeltrait for comparing label dominance - Implemented
compareinVertexLabelModel(always returnsGreaterto allow any override) andSOCLabelModel(compares SOC values) - Added
prune_treefunction in A* algorithm to remove dominated labels before tree insertion - Added helper methods to
SearchTree(get_labels_iter,get_labels_mut) andSearchTreeNode(traversal_cost) - Added new error types for label mismatches and missing nodes
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
rust/routee-compass-core/src/model/label/label_model.rs |
Adds compare method to LabelModel trait for dominance checking |
rust/routee-compass-core/src/model/label/default/vertex_label_model.rs |
Implements compare for default vertex labels, always returning Greater |
rust/routee-compass-powertrain/src/model/charging/soc_label_model.rs |
Implements compare for SOC labels by comparing SOC state values |
rust/routee-compass-core/src/algorithm/search/a_star/a_star_algorithm.rs |
Adds prune_tree function to remove dominated labels during search |
rust/routee-compass-core/src/algorithm/search/search_tree.rs |
Adds helper methods for label access and new error variant |
rust/routee-compass-core/src/algorithm/search/search_tree_node.rs |
Adds traversal_cost method to access node costs |
rust/routee-compass-core/src/model/label/label_model_error.rs |
Adds MismatchLabelTypes error variant for type mismatches |
rust/routee-compass-core/src/model/label/mod.rs |
Re-exports additional types for cleaner imports |
rust/routee-compass-core/src/algorithm/search/a_star/a_star_algorithm.rs
Outdated
Show resolved
Hide resolved
rust/routee-compass-core/src/algorithm/search/a_star/a_star_algorithm.rs
Outdated
Show resolved
Hide resolved
rust/routee-compass-core/src/algorithm/search/a_star/a_star_algorithm.rs
Outdated
Show resolved
Hide resolved
rust/routee-compass-core/src/algorithm/search/a_star/a_star_algorithm.rs
Outdated
Show resolved
Hide resolved
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 5 to 6. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v5...v6) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 6 to 7. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v6...v7) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
This reverts commit 7e0b3d4.
Collaborator
Author
|
@nreinicke this PR is once again ready for a review. latest changes:
|
nreinicke
reviewed
Feb 11, 2026
Collaborator
nreinicke
left a comment
nreinicke
left a comment
There was a problem hiding this comment.
This is looking good, thanks for adding this in. I think the logic is flipped for skipping the pruning but everything else looks good to me
| /// if not, then its type has a greater cardinality than the vertex set and so | ||
| /// we will want to prune any dominated labels with matching VertexId. | ||
| pub fn does_not_require_pruning(&self) -> bool { | ||
| !matches!(self, Label::Vertex(_)) |
Collaborator
There was a problem hiding this comment.
I think this logic is flipped right? We want to return True if self matches a Label::Vertex variant but this returns False for that variant.
nreinicke
approved these changes
Feb 11, 2026
Collaborator
nreinicke
left a comment
nreinicke
left a comment
There was a problem hiding this comment.
Dig it. Do you mind doing a squash merge on this one since the commit list is so big?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
this PR closes #445 by introducing a new compare method to LabelModel. it returns a std::cmp::Ordering designating whether a previous label is less than, greater than, or equal to some next label.
during A* directly before tree insert we prune the existing tree entries that share the same vertex. previous entries are removed if one dominates (edit) by minimizing cost and maximizing label state.
both LabelModel implementations were updated accordingly:
soc1.cmp(soc2))