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

Error during import - Failed to parse duration #10501

Closed
hamz2a opened this issue Jan 23, 2025 · 8 comments · Fixed by #10676
Closed

Error during import - Failed to parse duration #10501

hamz2a opened this issue Jan 23, 2025 · 8 comments · Fixed by #10676
Assignees
Labels
area:front Work on Standard OSRD Interface modules kind:bug Something isn't working module:operational-studies Multi-train simulation with structured studies management severity:critical Critical severity bug

Comments

@hamz2a
Copy link
Contributor

hamz2a commented Jan 23, 2025

What happened?

When importing data from open data with PLY as origin and LYD as destination for 23/01/2025, I received an error in the console:
"Failed to deserialize the JSON body into the target type: [6].schedule[0].arrival: Failed to parse duration: -PT30S at line 1 column 2584."
This error seems to mainly affect TER trains, where a negative duration (-PT30S) was not parsed correctly.

What did you expect to happen?

I expected the import to complete successfully without any errors.

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

  1. Go to scenario page.
  2. Select PLY as origin and LYD as destination.
  3. Set the date and time range to 23/01/2025 from 00:00 to 23:59.
  4. Click the magnifying glass to fetch the data.
  5. Click "Démarrer l'importation"
  6. The error will appear in the console.

On which environments the bug occurs?

Local

On which browser the bug occurs?

Google Chrome

OSRD version (top right corner Account button > Informations)

8d71c73

@hamz2a hamz2a added area:front Work on Standard OSRD Interface modules kind:bug Something isn't working module:operational-studies Multi-train simulation with structured studies management severity:critical Critical severity bug labels Jan 23, 2025
@Synar Synar self-assigned this Jan 31, 2025
@Synar
Copy link
Contributor

Synar commented Jan 31, 2025

I've added support for negative duration parsing, but I'm not sure I understand how to reproduce the bug. How can I get the open data in question on my local env? I've got no scenarios except those I created myself.

Such strings do not respect the ISO 8601 standard, which is why they were improperly parsed, but negativity is a common extension. Is there a reason why we get negative durations in arrival times though? This seems strange to me.

@clarani
Copy link
Contributor

clarani commented Feb 3, 2025

Hum I think the issue is that we should not have negative duration, there should be a bug at the creation of the Duration. Can you investigate this @Synar ?

@Synar
Copy link
Contributor

Synar commented Feb 3, 2025

I'm not sure we want to handle negative duration...

I mentioned this to @bougue-pe this morning, and he told me there might be cases where negative durations make sense. It was apparently discussed as a possible solution to model something (I'll let him explain =p), but he didn't know if it ended up being picked and implemented.

For our specific case, now that hamza explained to me how to reproduce it, I investigated, and it's (mostly) a path step duplication issue when importing the train list. I do not know if having more or less the same step twice is a data duplication error, or if it encapsulates some meaningful behavior. The proper fix probably depends on this question.

Image

Image

@Synar
Copy link
Contributor

Synar commented Feb 3, 2025

The data error if there is one come from Graou. If we consider this to be an error. we could either reject the schedule as valid in validateImportedTrainSchedules, or try to correct the schedule to remove duplicate steps.

There is also in the same schedule a mostly duplicated step with mostly empty info (see above) : is this valid data? In validateImportedTrainSchedules we check that uic/name are keys, but not whether their value are not empty. Should we? Or does it accurately represent a point on track with no name/uic?

@flomonster
Copy link
Contributor

It looks like a bug from Graou. I will ping @nicolaswurtz so he can check from his side.
Meanwhile, in my opinion, we can skip these invalid steps.

@clarani
Copy link
Contributor

clarani commented Feb 4, 2025

  • for the first point, I agree with @flomonster

  • for the empty step, I think that the data may be valid if we have at least a uic or a trigram (which is the case here), so we should handle it.

(The function validateImportedTrainSchedules was written very quickly and may be improved indeed)

@Synar
Copy link
Contributor

Synar commented Feb 4, 2025

@flomonster @clarani If we consider that the step with empty name is valid, then the issue is not just step duplication, but also times duplications between different steps.

So if we want to prevent negative durations by skipping steps, we would skip Joigny entirely in my second screenshot for example.

For now I'm dropping the other PR and implementing this instead.

@Synar
Copy link
Contributor

Synar commented Feb 4, 2025

Additional issues with the data imported from graou :

  • most trains do not have a rolling stock selected, this can easily be fixed by the user
  • modifying the code to properly support steps with only trigrams shows that the corresponding point is not at all on the path (and leads to a canton missing error). So I'm not sure if we should handle the trigram only steps after all, or if it would be better to skip.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:front Work on Standard OSRD Interface modules kind:bug Something isn't working module:operational-studies Multi-train simulation with structured studies management severity:critical Critical severity bug
Projects
None yet
4 participants