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

[LTS/1.8.x] NaN saved in output #490

Closed
riccioc opened this issue Apr 25, 2024 · 4 comments · Fixed by #518
Closed

[LTS/1.8.x] NaN saved in output #490

riccioc opened this issue Apr 25, 2024 · 4 comments · Fixed by #518
Labels
bug ❌ Something isn't working LTS/1.8 Issue applies to Long Term Support for 1.8

Comments

@riccioc
Copy link
Collaborator

riccioc commented Apr 25, 2024

Running a fit NaN values are saved for the pion FSI parameters. This was not observed before and may be related to the configuration files, but it could be avoided by adding a protection against it. I have identified the following lines where this protection can go: L943 and L944 in MinimizerInterface.cpp. I was thinking of adding a simple if(not std::isnan(par.getParameterValue()) ) before setting the bin content and if( not std::isnan((*covMatrix_)[par.getParameterIndex()][par.getParameterIndex()]) ) before setting the parameter errors. The exception can be that the bin is set to zero and an error is printed in the log. Feedback on my proposal is welcome!

@tadoyle
Copy link
Collaborator

tadoyle commented Apr 26, 2024

I thinking seeing NaN is a very clear indication something went wrong; unless this causes crashes I'd be tempted to leave it so the underlying problem isn't missed. If it does cause issues, I think an exception like you propose above works. I'd probably set the value to something like -999 so it's still very obvious that something went wrong, 0 could be overlooked if people don't check carefully.

@ClarkMcGrew
Copy link
Contributor

ClarkMcGrew commented Apr 26, 2024

I pushed a test branch LTS/testbed/OnlyValidParameters that will throw if a NaN parameter is used. It should also print the name of the parameter. I would go further than @tadoyle and say using any invalid parameters should be treated as a full "cannot continue" situation.

@ClarkMcGrew ClarkMcGrew added bug ❌ Something isn't working LTS/1.8 Issue applies to Long Term Support for 1.8 labels May 1, 2024
@riccioc
Copy link
Collaborator Author

riccioc commented May 2, 2024

I've tested this version and it throws an error as expected what(): exception thrown by the logger: std::isnan(_parameterValue_)

@ClarkMcGrew
Copy link
Contributor

ClarkMcGrew commented May 15, 2024

Based on discussions: The actual issue is that disabled parameters are being saved into the output. Disabled parameters should not be saved. If there are ENABLED parameters with a NaN value, that should be an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ❌ Something isn't working LTS/1.8 Issue applies to Long Term Support for 1.8
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants