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: we no longer anticipate the speed limits #10136

Closed
Castavo opened this issue Dec 19, 2024 · 3 comments · Fixed by #10143
Closed

core: we no longer anticipate the speed limits #10136

Castavo opened this issue Dec 19, 2024 · 3 comments · Fixed by #10143
Assignees
Labels
area:core Work on Core Service kind:bug Something isn't working severity:major Major severity bug

Comments

@Castavo
Copy link
Contributor

Castavo commented Dec 19, 2024

What happened?

Trains respect speed limit at the closest.

What did you expect to happen?

Trains should respect lowering of speed limits 100m before their actual start.

This was present in the version of simulations for train schedule V1, and was dropped.

As seen with @Arthur-Lefebvre, @axrolld and @Tguisnet, it is expected that simulations apply this 100m margin.

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

  1. Make a simulation and witness that the breaking curves end right to the speed limit decreases

On which environments the bug occurs?

Recette (SNCF)

On which browser the bug occurs?

Firefox

OSRD version (top right corner Account button > Informations)

6580cfe

@Castavo Castavo added area:core Work on Core Service kind:bug Something isn't working severity:major Major severity bug labels Dec 19, 2024
@Castavo Castavo changed the title core: reintroduce the 100m anticipation of speed limits core: we no longer anticipate the speed limits Dec 19, 2024
@eckter eckter self-assigned this Dec 19, 2024
@eckter
Copy link
Contributor

eckter commented Jan 15, 2025

So according to @loic-hamelin, we don't actually want this in the current running time computations. It's not relevant now and we'll only think about this with the v3. Apparently it's wrong to apply it in TVM or ERTMS contexts.

@Castavo @Arthur-Lefebvre, @axrolld and @Tguisnet: does it sound good to you? would you be OK with me reverting #10143?

@Arthur-Lefebvre
Copy link

I agree with the fact that it doesn't apply in TVM and ERTMS contexts, but it applies in all others.

When I have to modelize it, I treat this as a driver behaviour. It's usual that the driver behaviour depends of the signalling system.

If it's not possible to make th difference now and I had to choose, I'd rather keep this PR but both are possible.

@Castavo
Copy link
Contributor Author

Castavo commented Jan 15, 2025

I don't care whether we keep it or not.
Maybe we should ask some of our clients first ? We'll discuss about this and come back to you, thanks for letting us know

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:major Major severity bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants