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, editoast, python: stop train on next signal instead of OP #10200

Draft
wants to merge 10 commits into
base: dev
Choose a base branch
from

Conversation

louisgreiner
Copy link
Contributor

@louisgreiner louisgreiner commented Dec 27, 2024

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

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

image

With a 83500, 109m train

image

@github-actions 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
@codecov-commenter
Copy link

codecov-commenter commented Dec 27, 2024

⚠️ Please install the 'codecov app svg image' 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.47%. Comparing base (995c248) to head (0a7d46d).
Report is 5 commits behind head on dev.

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.49%   81.47%   -0.02%     
==========================================
  Files        1058     1041      -17     
  Lines      104270   103635     -635     
  Branches      722      722              
==========================================
- Hits        84970    84435     -535     
+ Misses      19259    19159     -100     
  Partials       41       41              
Flag Coverage Δ
editoast 73.71% <100.00%> (-0.03%) ⬇️
front 89.24% <0.00%> (+0.03%) ⬆️
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 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 github-actions bot added the area:front Work on Standard OSRD Interface modules label Jan 6, 2025
@louisgreiner louisgreiner force-pushed the lgr/fix-train-switch-overflow branch 5 times, most recently from a3b0127 to c06ea7a Compare January 7, 2025 13:37
@louisgreiner louisgreiner force-pushed the lgr/fix-train-switch-overflow branch 3 times, most recently from e428743 to 5a5ff11 Compare January 8, 2025 13:33
Signed-off-by: Louis Greiner <[email protected]>
@louisgreiner louisgreiner force-pushed the lgr/fix-train-switch-overflow branch from 5a5ff11 to 18a57e2 Compare January 8, 2025 13:46
Signed-off-by: Louis Greiner <[email protected]>
Copy link
Contributor Author

@louisgreiner louisgreiner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment on lines +18 to +19
#[derivative(Default(value = "true"))]
#[serde(default = "default_stop_at_next_signal")]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment on lines +27 to +29
fn default_stop_at_next_signal() -> bool {
true
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fn default_stop_at_next_signal() -> bool {
true
}

@@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@louisgreiner
Copy link
Contributor Author

Questions :

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

Signed-off-by: Louis Greiner <[email protected]>
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 area:editoast Work on Editoast Service area:front Work on Standard OSRD Interface modules area:railjson Work on Proposed Unified Rail Assets Data Exchange Format
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants