core, editoast, python: stop train on next signal instead of OP #10200

louisgreiner · 2024-12-27T13:38:27Z

Closes https://github.com/osrd-project/osrd-confidential/issues/892
Closes https://github.com/osrd-project/osrd-confidential/issues/830

Depends on https://gitlab-repo-res.apps.eul.sncf.fr/dsir/groupedxs-dsir/04735/etl_basic/-/merge_requests/145 for internal use

Analysis of the solution, compared to the default behavior can be seen here

Description

Note : the following screenshots are for review visualization only, nothing will be output in the front-end at the end

The train stops are now shifted the least possible to the next signal position, with a minimal value that would put the tail of the train on the operational point.

Examples

With a `TGV2N2`, 400m train

With a `83500`, 109m train

codecov-commenter · 2024-12-27T13:40:46Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 81.54%. Comparing base (9ddc06c) to head (9f36ef1).

Files with missing lines	Patch %	Lines
...alStudies/hooks/useSetupItineraryForTrainUpdate.ts	0.00%	1 Missing ⚠️
front/src/modules/pathfinding/utils.ts	0.00%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev   #10200      +/-   ##
==========================================
- Coverage   81.60%   81.54%   -0.07%     
==========================================
  Files        1067     1049      -18     
  Lines      105535   104711     -824     
  Branches      727      727              
==========================================
- Hits        86126    85388     -738     
+ Misses      19368    19282      -86     
  Partials       41       41

Flag	Coverage Δ
editoast	`73.61% <100.00%> (-0.06%)`	⬇️
front	`89.30% <0.00%> (-0.01%)`	⬇️
gateway	`2.18% <ø> (ø)`
osrdyne	`3.28% <ø> (ø)`
railjson_generator	`87.50% <ø> (ø)`
tests	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

louisgreiner

We keep this flag to true for review purpose, but before merging, we need to set it to false

louisgreiner · 2025-01-08T15:53:16Z

editoast/editoast_schemas/src/train_schedule/train_schedule_options.rs

+    #[derivative(Default(value = "true"))]
+    #[serde(default = "default_stop_at_next_signal")]


Suggested change

#[derivative(Default(value = "true"))]

#[serde(default = "default_stop_at_next_signal")]

#[derivative(Default(value = "false"))]

louisgreiner · 2025-01-08T15:53:34Z

editoast/editoast_schemas/src/train_schedule/train_schedule_options.rs

+fn default_stop_at_next_signal() -> bool {
+    true
+}


Suggested change

fn default_stop_at_next_signal() -> bool {

true

}

louisgreiner · 2025-01-08T15:53:54Z

front/src/applications/operationalStudies/hooks/useSetupItineraryForTrainUpdate.ts

@@ -149,6 +149,7 @@ const useSetupItineraryForTrainUpdate = (trainIdToEdit: number) => {
          rolling_stock_supported_signaling_systems: rollingStock.supported_signaling_systems,
          rolling_stock_maximum_speed: rollingStock.max_speed,
          rolling_stock_length: rollingStock.length,
+          stop_at_next_signal: true, // TODO: change to false and set to true elsewhere if needed


Suggested change

stop_at_next_signal: true, // TODO: change to false and set to true elsewhere if needed

stop_at_next_signal: false,

louisgreiner · 2025-01-08T15:54:10Z

front/src/modules/pathfinding/utils.ts

@@ -79,6 +79,7 @@ export const getPathfindingQuery = ({
        rolling_stock_supported_signaling_systems: rollingStock.supported_signaling_systems,
        rolling_stock_maximum_speed: rollingStock.max_speed,
        rolling_stock_length: rollingStock.length,
+        stop_at_next_signal: true, // TODO: change to false and set to true elsewhere if needed


Suggested change

stop_at_next_signal: true, // TODO: change to false and set to true elsewhere if needed

stop_at_next_signal: false,

louisgreiner · 2025-01-08T16:16:53Z

Questions :

Currently it moves all the waypoints of the train schedules instead of only the stops ; is it a real problem ?

eckter

That's the right approach, good job 👍

core/src/main/kotlin/fr/sncf/osrd/api/api_v2/pathfinding/PathfindingBlocksEndpointV2.kt

louisgreiner · 2025-01-15T10:39:09Z

Question : is the type of signalisation (here TVM instead of BAL) could affect the behaviour of the code ?

Because here, the waypoint is moved AFTER the signal, which is not correct (but I only see that behaviour on TVM signals)

Could it be also related to the current bug that breaks the projection of intermediary waypoints on the map ?

Edit: looks like I was right, but difficult to be sure (the position here is now correct for the end pathstep)

Signed-off-by: Louis Greiner <[email protected]>

…serializer Signed-off-by: Louis Greiner <[email protected]>

Signed-off-by: Louis Greiner <[email protected]>

github-actions bot added area:railjson Work on Proposed Unified Rail Assets Data Exchange Format area:core Work on Core Service area:editoast Work on Editoast Service labels Dec 27, 2024

louisgreiner force-pushed the lgr/fix-train-switch-overflow branch 3 times, most recently from 03bbd62 to 101da24 Compare December 27, 2024 17:29

github-actions bot added the area:front Work on Standard OSRD Interface modules label Jan 6, 2025

louisgreiner force-pushed the lgr/fix-train-switch-overflow branch 9 times, most recently from 5a5ff11 to 18a57e2 Compare January 8, 2025 13:46

louisgreiner commented Jan 8, 2025

View reviewed changes

eckter reviewed Jan 9, 2025

View reviewed changes

core/src/main/kotlin/fr/sncf/osrd/api/api_v2/pathfinding/PathfindingBlocksEndpointV2.kt Outdated Show resolved Hide resolved

louisgreiner force-pushed the lgr/fix-train-switch-overflow branch 2 times, most recently from 70217a1 to bac5a63 Compare January 14, 2025 09:31

louisgreiner force-pushed the lgr/fix-train-switch-overflow branch from bac5a63 to 03012e2 Compare January 15, 2025 13:51

louisgreiner added 6 commits January 15, 2025 16:46

editoast, python: add ts.config 'stop_at_next_signal' new field

c02c92c

Signed-off-by: Louis Greiner <[email protected]>

core: move block offset to next signal if flag

7b9c269

Signed-off-by: Louis Greiner <[email protected]>

front, editoast: set stop_at_next_signal from front instead of serde …

a35a326

…serializer Signed-off-by: Louis Greiner <[email protected]>

core: move block offset to next signal in stdcm

609b801

Signed-off-by: Louis Greiner <[email protected]>

core: remove from standaloneSimulation

af5e998

Signed-off-by: Louis Greiner <[email protected]>

core: lint

b69e5d0

Signed-off-by: Louis Greiner <[email protected]>

louisgreiner added 6 commits January 15, 2025 16:46

core: add stopAtNextSignalTest

bfc6e56

Signed-off-by: Louis Greiner <[email protected]>

core: fix ci

5226017

Signed-off-by: Louis Greiner <[email protected]>

core: remove flag from stdcm

c44371b

Signed-off-by: Louis Greiner <[email protected]>

core: lint

0cabaef

Signed-off-by: Louis Greiner <[email protected]>

core: add tests for reversed itineraries

53e8012

Signed-off-by: Louis Greiner <[email protected]>

core: remove block borders condition

9f36ef1

Signed-off-by: Louis Greiner <[email protected]>

louisgreiner force-pushed the lgr/fix-train-switch-overflow branch from 03012e2 to 9f36ef1 Compare January 15, 2025 15:46

louisgreiner marked this pull request as ready for review January 16, 2025 10:32

louisgreiner requested review from a team as code owners January 16, 2025 10:32

louisgreiner requested a review from Khoyo January 16, 2025 10:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core, editoast, python: stop train on next signal instead of OP #10200

core, editoast, python: stop train on next signal instead of OP #10200

louisgreiner commented Dec 27, 2024 •

edited

Loading

codecov-commenter commented Dec 27, 2024 •

edited

Loading

louisgreiner left a comment

louisgreiner Jan 8, 2025

louisgreiner Jan 8, 2025

louisgreiner Jan 8, 2025

louisgreiner Jan 8, 2025

louisgreiner commented Jan 8, 2025

eckter left a comment

louisgreiner commented Jan 15, 2025 •

edited

Loading

		#[derivative(Default(value = "true"))]
		#[serde(default = "default_stop_at_next_signal")]

	#[derivative(Default(value = "true"))]
	#[serde(default = "default_stop_at_next_signal")]
	#[derivative(Default(value = "false"))]

	stop_at_next_signal: true, // TODO: change to false and set to true elsewhere if needed
	stop_at_next_signal: false,

core, editoast, python: stop train on next signal instead of OP #10200

Are you sure you want to change the base?

core, editoast, python: stop train on next signal instead of OP #10200

Conversation

louisgreiner commented Dec 27, 2024 • edited Loading

Description

Examples

With a TGV2N2, 400m train

With a 83500, 109m train

codecov-commenter commented Dec 27, 2024 • edited Loading

Codecov Report

louisgreiner left a comment

Choose a reason for hiding this comment

louisgreiner Jan 8, 2025

Choose a reason for hiding this comment

louisgreiner Jan 8, 2025

Choose a reason for hiding this comment

louisgreiner Jan 8, 2025

Choose a reason for hiding this comment

louisgreiner Jan 8, 2025

Choose a reason for hiding this comment

louisgreiner commented Jan 8, 2025

eckter left a comment

Choose a reason for hiding this comment

louisgreiner commented Jan 15, 2025 • edited Loading

louisgreiner commented Dec 27, 2024 •

edited

Loading

With a `TGV2N2`, 400m train

With a `83500`, 109m train

codecov-commenter commented Dec 27, 2024 •

edited

Loading

louisgreiner commented Jan 15, 2025 •

edited

Loading