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

refacto: stdcm: unify stop data #8974

Open
3 tasks done
eckter opened this issue Sep 24, 2024 · 1 comment · May be fixed by #10989
Open
3 tasks done

refacto: stdcm: unify stop data #8974

eckter opened this issue Sep 24, 2024 · 1 comment · May be fixed by #10989
Assignees
Labels
area:core Work on Core Service kind:refacto-task Task related to Refactorization Epic module:stdcm Short-Term DCM

Comments

@eckter
Copy link
Contributor

eckter commented Sep 24, 2024

Currently, the stop data are spread all over the place. They are:

  1. In TimeData (min/max/current stop durations)
  2. In InfraExplorerWithEnvelope (position / current duration)
  3. In InfraExplorer (list of all possible stop locations)

This is a mess. It happened because we couldn't plan early enough with a clear vision of how those features would evolve. We should ideally merge the 1 and 2. The third one is a bit different as it's not just about the explored path, but it can still make new stop-related features more difficult.

This refactoring would take some planning workshops, the end goal isn't clear yet.

Note: the current situation is functional for now. It's just some tech debt, debugging is more difficult and new features may be limited.


Likely a requirement for:

  1. Adding stops to avoid conflicts (https://github.com/osrd-project/osrd-confidential/issues/338)
  2. Proper safety speed management (stdcm: add safety speed ranges in stdcm #9282)

Definition of ready

  • Technical

    • Implementation plan is detailed and validated by maintainers
  • General

    • WSJF values are filled
    • Validated by the PM
@eckter eckter added area:core Work on Core Service module:stdcm Short-Term DCM kind:refacto-task Task related to Refactorization Epic labels Sep 24, 2024
@eckter
Copy link
Contributor Author

eckter commented Oct 22, 2024

Notes:

  • We should probably also rework how we store the input stop data and how we figure out when we need to add stops

List of constraints and why we did it that way:

  • TimeData values: this is the place where we edit the most the stop data, where we make them longer or shorter. This structure is a nice place to store this data.
  • InfraExplorerWithEnvelope:
    • We also need the stop offsets
    • Stops need to be part of the conflict detection inputs. This class is supposed to generate the conflict detection inputs.
    • We need to build an EnvelopeStopWrapper (or something with a similar behavior)
  • InfraExplorer:
    • We need to list stops in PathFragments
    • We need the path start and end

Some partial conclusions:

  • We need to figure out if/when we stop as we explore the path, not when we simulate that part
    • We also need to figure out if the stop is on a closed signal
    • (We can just always stop on closed signals)
  • We could model it as a stop of infinite duration and we would edit it later on. But we need to carefully check that the spacing resource generator doesn't cache the wrong stuff.
    • We may just not cache ressource use if there's an infinite-length stop
    • This matches the use case of pendingRequirements, we can see if it can handle it (with or without change)

TODO?

  • Goal: keep stop durations in TimeData, keep stop offsets in InfraExplorer. Define stops as we extend the InfraExplorer's lookahead.
  • To remove the InfraExplorerWithEnvelope part:
    • We can move stop offsets and move them (to InfraExplorer maybe?)
    • Stop as inputs to the conflict detection: have to be handled by the InfraExplorer (part of IncrementalPath)
    • We have all the info we need to build the EnvelopeStopWrapper, using TimeData
  • InfraExplorer
    • Improve how we find stop locations when extending infra explorers (TODO, maybe create a new class to handle it? It would handle closed signal decision)

@axrolld axrolld changed the title refacto: stdcm: unify stop data Unifier les informations relatives aux arrêts dans LMR Nov 5, 2024
@axrolld axrolld changed the title Unifier les informations relatives aux arrêts dans LMR refacto: stdcm: unify stop data Nov 5, 2024
@eckter eckter removed their assignment Nov 15, 2024
@eckter eckter self-assigned this Jan 29, 2025
@eckter eckter linked a pull request Mar 3, 2025 that will close this issue
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:refacto-task Task related to Refactorization Epic module:stdcm Short-Term DCM
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant