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

[Bug/Question]: Off-Momentum RDT/CRDT calculation #456

Open
JoschD opened this issue Sep 13, 2024 · 1 comment
Open

[Bug/Question]: Off-Momentum RDT/CRDT calculation #456

JoschD opened this issue Sep 13, 2024 · 1 comment
Assignees
Labels
Estimate: Easy Good first issue for newcomers. Straightforward fixes. Priority: Medium Work on this. Status: In Progress Currently being worked on. Type: Bug Something isn't working as it should. Type: Question Something is not clear or is not understood.

Comments

@JoschD
Copy link
Member

JoschD commented Sep 13, 2024

Bug Description

I found a bug where the (C)RDT calculation fails when analysing either purely off-momentum files, or both on- and off- momentum files together.
The reason is, that it is hard-coded to take the phase model values only from on-momentum files, but the phase measurement values from all files.

This either leads to an error when only off-momentum files are analysed, as no model files can be found,
or to a dimension mismatch later on when both on- and off-momentum files are analysed.

I have now implemented a simple switch in #455 that only takes on-momentum files for both, model and measurement.
I.e. we forbid (C)RDT analysis for off-momentum files (raises error) and ignore off-momentum files when doing a combined analysis (only uses on-momentum files to calculate (C)RDTs).

Is this the behaviour we want, or would it make sense to include all, on- and off-momentum in the analysis?

This question goes out to @mihofer @fscarlier @emaclean @rogeliotomas and @Mael-Le-Garrec

@JoschD JoschD added Type: Bug Something isn't working as it should. Estimate: Easy Good first issue for newcomers. Straightforward fixes. Type: Question Something is not clear or is not understood. Priority: Medium Work on this. Status: Review Needed Work currently stopped, untils someone else reviews it. labels Sep 13, 2024
@JoschD JoschD self-assigned this Sep 13, 2024
@JoschD JoschD changed the title [Bug/Question]: Off-Momentum CRDT calculation [Bug/Question]: Off-Momentum RDT/CRDT calculation Sep 13, 2024
@JoschD
Copy link
Member Author

JoschD commented Sep 17, 2024

After discussion with Ewen: Use all files, but warn user that he is using off-momentum files for RDT calculation.

@JoschD JoschD added Status: In Progress Currently being worked on. and removed Status: Review Needed Work currently stopped, untils someone else reviews it. labels Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Estimate: Easy Good first issue for newcomers. Straightforward fixes. Priority: Medium Work on this. Status: In Progress Currently being worked on. Type: Bug Something isn't working as it should. Type: Question Something is not clear or is not understood.
Projects
None yet
Development

No branches or pull requests

1 participant