Fix resolution function numerical issue

hmcezar commented 6 months ago

Description

The way the resolution function was computed in the SAXS/SANS action could lead to some numerical issues due to computing the modified Bessel function with a large argument. This could cause some inf and NaN depending on the sigmas and qs.

I have fixed this issue by implementing the cephes function i0e, which computes I_0(x) exp(-x) and is less prone to overflow.

Target release

I would like my code to appear in release 2.9

Type of contribution

[X] changes to code or doc authored by PLUMED developers, or additions of code in the core or within the default modules
[ ] changes to a module not authored by you
[ ] new module contribution or edit of a module authored by you

Copyright

[X] I agree to transfer the copyright of the code I have written to the PLUMED developers or to the author of the code I am modifying.

[ ] the module I added or modified contains a COPYRIGHT file with the correct license information. Code should be released under an open source license. I also used the command cd src && ./header.sh mymodulename in order to make sure the headers of the module are correct.

Tests

[ ] I added a new regtest or modified an existing regtest to validate my changes.
[X] I verified that all regtests are passed successfully on GitHub Actions.

codecov-commenter commented 6 months ago

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (5cbb885) 84.09% compared to head (7b4b946) 84.08%.

:exclamation: Current head 7b4b946 differs from pull request most recent head 4126b19. Consider uploading reports for the commit 4126b19 to get more accurate results

Files	Patch %	Lines
src/isdb/SAXS.cpp	76.92%	3 Missing :warning:

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

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #1002 +/- ## ========================================== - Coverage 84.09% 84.08% -0.01% ========================================== Files 602 602 Lines 56233 56246 +13 ========================================== + Hits 47287 47297 +10 - Misses 8946 8949 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

carlocamilloni commented 6 months ago

@hmcezar sorry I merged and reverted it because you should target v2.9 if you want this to appear there

hmcezar commented 6 months ago

@carlocamilloni I think it was my mistake. The current 2.9 does not have the "old" implementation of the resolution function and your most recent fixes. I think it's better if it goes to master to be consistent with what we already have. Should I create another PR?

carlocamilloni commented 6 months ago

ok, I have reverted the revert, not the best but it is done

hmcezar commented 6 months ago

Thank you! And sorry for the mess. My bad.

plumed / plumed2