plumed / plumed2

Development version of plumed 2
https://www.plumed.org
GNU Lesser General Public License v3.0
357 stars 283 forks source link

Patched the rational non simplified switching function around X==R_0 #1048

Closed Iximiel closed 6 months ago

Iximiel commented 6 months ago
Description

First of all, sorry for the Friday afternoon PR.

As we discussed in #1038, here I patched the numerical instabilities around R0 in the non simplified rational switching function. And I used the test from #1038 for ensuring the correctness of the change (v2.8 won't pass the new test).

Before merging: should I update the changelog?

Target release

I would like my code to appear in release v2.8

Type of contribution
Copyright
Tests
codecov-commenter commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 85.90%. Comparing base (221032c) to head (9ccf5b9).

:exclamation: Current head 9ccf5b9 differs from pull request most recent head 8768f05. Consider uploading reports for the commit 8768f05 to get more accurate results

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

Additional details and impacted files ```diff @@ Coverage Diff @@ ## v2.8 #1048 +/- ## ========================================== + Coverage 85.87% 85.90% +0.03% ========================================== Files 595 595 Lines 49748 49752 +4 ========================================== + Hits 42720 42741 +21 + Misses 7028 7011 -17 ```

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

GiovanniBussi commented 6 months ago

Great! I see that other tests are not affected, I am afraid there was no test on the 2 N!= M case, or maybe no test was probing that point?

I would add something in the change log for version 2.8

Thanks!

Iximiel commented 6 months ago

I am afraid there was no test on the 2 N!= M case, or maybe no test was probing that point?

I think that was the reason it slipped trough the tests. We have to thank intel that approximates differently than gcc

I update the changelog, I think this can be squashed+merged