plumed / plumed2

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

Removed call to set that is used in DomainDecomposition. #1026

Closed gtribello closed 7 months ago

gtribello commented 7 months ago

This addresses issues #1025 @GiovanniBussi

Description

This commit removes the call to set that is discussed in issue #1025. I don't see that this making a noticeable difference to performance when compared with master but I am possibly not running on the correct setup.

timings

Target release

I would like my code to appear in release 2.10

Type of contribution
Copyright
Tests
carlocamilloni commented 7 months ago

@gtribello @gbussi on my big system (where I use 4 mpi) this is actually slower than master this: PLUMED: 3 Waiting for data 2001 1.161034 master: PLUMED: 3 Waiting for data 2001 0.713679

codecov-commenter commented 7 months ago

Codecov Report

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

Comparison is base (9faf503) 84.11% compared to head (f7be61b) 84.11%.

:exclamation: Current head f7be61b differs from pull request most recent head 5424858. Consider uploading reports for the commit 5424858 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 @@ ## master #1026 +/- ## ========================================== - Coverage 84.11% 84.11% -0.01% ========================================== Files 614 614 Lines 56664 56663 -1 ========================================== - Hits 47664 47663 -1 Misses 9000 9000 ```

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

GiovanniBussi commented 7 months ago

@carlocamilloni I think it's impossible. Are you sure you are comparing the right versions?

I mean: this change alone basically replaces a function call which does something + sets an element in an array with an explicit instruction that directly sets that element. Am I right @gtribello ?

gtribello commented 7 months ago

I agree @GiovanniBussi. The fact that it is slower is very strange.

carlocamilloni commented 7 months ago

yes you are right, with mpi in comparison to openMP (that is how I usually ran this test) the variability is much larger on the 2,000 steps that i use for this test. In this respect the branch does not make any significant difference