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

Core: DistanceRangeMap.putMany() doesn't merge equal values for consecutive intervals #6726

Closed
bougue-pe opened this issue Feb 23, 2024 · 1 comment · Fixed by #6731
Closed
Assignees
Labels
area:core Work on Core Service kind:bug Something isn't working severity:minor Minor severity bug

Comments

@bougue-pe
Copy link
Contributor

What happened?

See tests in /~https://github.com/osrd-project/osrd/compare/peb/core/fix_distance_range_put_many
The merge when extremities and values are the same is not done by putMany(), leading to a different behavior than expected, and than put().

What did you expect to happen?

Have putMany() merging consecutive ranges with identical values.

How can we reproduce it (as minimally and precisely as possible)?

Use tests available in peb/core/fix_distance_range_put_many branch

What operating system, browser and environment are you using?

  • WSL
  • Gradle
  • Java 17

OSRD version (top right corner Account button > Informations)

dfcbaad

@bougue-pe bougue-pe added kind:bug Something isn't working area:core Work on Core Service severity:minor Minor severity bug labels Feb 23, 2024
@shenriotpro
Copy link
Contributor

IMO the main issue is that putMany considers equal bounds separately /~https://github.com/osrd-project/osrd/blob/ac93bc8040d8e558a2f5350ad022ee347772df2e/core/kt-osrd-utils/src/main/kotlin/fr/sncf/osrd/utils/DistanceRangeMapImpl.kt#L60

We could use mergeAdjacent for a quick fix, but I think it would be more efficient/readable to group bounds in putMany.
We may also want to split the logic in two parts: e.g. the first part handles groups (removes empty intervals) and the second part merges adjacent intervals with the same value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:core Work on Core Service kind:bug Something isn't working severity:minor Minor severity bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants