Add ability to specify a length-invariant cost layer #173
Conversation
ppinchuk
added
enhancement
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #173 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 17 17
Lines 1392 1392
Branches 168 168
=========================================
Hits 1392 1392
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds support for length-invariant cost layers in reVRt's routing engine. These layers represent one-time transition costs that don't scale with the distance traveled through a cell (e.g., a fixed cost for transitioning from underwater to above-ground cable routes along a coastline).
Key Changes:
- Extended
CostLayerstruct with anis_invariantfield to mark layers as distance-independent - Refactored cost calculation to compute and store both invariant and distance-dependent costs in separate layers (
/cost_invariantand/cost) - Updated neighbor cost retrieval to combine averaged distance-dependent costs with unscaled invariant costs
- Activated and updated previously dead tests, adding comprehensive test coverage for the new invariant layer functionality
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/revrt/src/dataset/lazy_subset.rs | Fixed spelling errors in documentation ("unecessary" → "unnecessary", "assumtions" → "assumptions") |
| crates/revrt/src/dataset.rs | Core implementation: added dual-layer cost storage, refactored cost calculation to handle both invariant and distance-scaled layers, extracted add_layer_to_data and get_neighbor_costs helper functions, and updated tests to verify correct behavior |
| crates/revrt/src/cost.rs | Extended CostLayer with is_invariant field, updated compute method to filter layers by type, refactored layer building logic into separate function, and returns zero-filled arrays when no matching layers exist |
|
@castelao this is ready for review |
castelao
left a comment
castelao
left a comment
There was a problem hiding this comment.
@ppinchuk , thanks for moving this ahead. Just to be clear, I'm aware that some of my comments are related with inherited code that I did it myself, i.e. I'm aware that it was my own fault. I would do a couple of things differently, but let's move this ahead and we can handle in smaller components in future PRs once I'm able to get back to this. Now that this system is a little bit better split in components, it will be easier to progress in small parts.
Thanks!
| } | ||
| } | ||
|
|
||
| fn add_layer_to_data( |
There was a problem hiding this comment.
This function should return a Result, so we could replace .unwrap() for ?. When parallelizing, we don't want that one failure to crash everything.
That said, there is so much still to improve that is totaly fine to leave it as it is for now and we will handle that later.
| let cost_function = | ||
| CostFunction::from_json(r#"{"cost_layers": [{"layer_name": "A"}]}"#).unwrap(); | ||
| let dataset = | ||
| Dataset::open(path, cost_function, 250_000_000).expect("Error opening dataset"); |
There was a problem hiding this comment.
It shouldn't reach ~250MB, but since it is a test, let's play on the safe side and reduce this a lot?
|
To saver time, I will merge now and add these suggestions into the other PR |
One functionality we have in reVX that wasn't in reVRt yet is to have layers that don't get scaled by the distanced traversed in each cell.
For example, imagine you are moving a cable from underwater to above-ground. There is a one-time cost you must pay for that transition along the coast. This cost should not be scaled by the distance in the cell - it's juts a transition cost.
In this PR, I added functionality to the
CostFunctionto handle such layers. This includes a few new tests as well as fixing up older tests that were marked as dead code.