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

Some benchmarks improvements #133

Merged
merged 1 commit into from
Apr 8, 2017
Merged

Some benchmarks improvements #133

merged 1 commit into from
Apr 8, 2017

Conversation

Luthaf
Copy link
Member

@Luthaf Luthaf commented Apr 2, 2017

This PR adds the current file name to the benchmark name for better readability of the output.
It also move all expensive setup to a lazy_static! macro, to try reducing the variability of benchmarks, but I am not totally convinced about this part.

The first two commits are to be discussed independently in #132.

The results are not really convincing, but I am sharing this to gather feedback.

argon::cache_move_particle                        169400 (+/- 54.85%)     221268 (+/- 43.01%) ∆ = -11.84%
argon::energy                                    2207974 (+/- 18.07%)    2031256 (+/- 42.66%) ∆ = +24.60%
argon::forces                                    2514489 (+/- 18.12%)    2492820 (+/- 20.65%) ∆ =  +2.53%
argon::virial                                    2894937 (+/- 16.04%)    3046181 (+/- 19.33%) ∆ =  +3.29%
nacl::cache_move_particle_ewald                   326826 (+/- 30.16%)     301871 (+/- 22.98%) ∆ =  -7.18%
nacl::cache_move_particle_wolf                     90758 (+/- 83.55%)      97785 (+/- 26.48%) ∆ = -57.06%
nacl::energy_ewald                                511988 (+/- 44.58%)     566218 (+/- 45.89%) ∆ =  +1.31%
nacl::energy_wolf                                 408295 (+/- 29.48%)     440044 (+/- 28.73%) ∆ =  -0.74%
nacl::forces_ewald                              13173069 (+/- 14.56%)   13663057 (+/- 16.82%) ∆ =  +2.26%
nacl::forces_wolf                                 564239 (+/- 19.97%)     543863 (+/- 45.88%) ∆ = +25.91%
nacl::virial_ewald                              58961445 (+/-  9.74%)   58885824 (+/- 16.98%) ∆ =  +7.24%
nacl::virial_wolf                                 577238 (+/- 21.37%)     516429 (+/- 28.58%) ∆ =  +7.21%
propane::cache_move_all_rigid_molecules          1179527 (+/- 26.13%)    2662206 (+/- 20.02%) ∆ =  -6.12%
propane::cache_move_particles                     261200 (+/- 31.54%)     220522 (+/- 25.82%) ∆ =  -5.72%
propane::energy                                  1238951 (+/- 30.58%)    1163063 (+/- 39.72%) ∆ =  +9.14%
propane::forces                                  1518188 (+/- 28.71%)    1418140 (+/- 27.05%) ∆ =  -1.66%
propane::virial                                  1529583 (+/- 24.99%)    1333815 (+/- 22.34%) ∆ =  -2.64%
water::cache_move_all_rigid_molecules_ewald      7974863 (+/- 21.58%)    7703067 (+/- 17.18%) ∆ =  -4.40%
water::cache_move_all_rigid_molecules_wolf       7679899 (+/- 15.91%)    7905734 (+/- 19.01%) ∆ =  +3.10%
water::cache_move_particles_ewald                 667924 (+/- 35.80%)     766619 (+/- 28.75%) ∆ =  -7.05%
water::cache_move_particles_wolf                  396990 (+/- 34.02%)     469701 (+/- 21.42%) ∆ = -12.60%
water::energy_ewald                               858081 (+/- 68.53%)     928018 (+/- 20.17%) ∆ = -48.36%
water::energy_wolf                                648018 (+/- 62.34%)     678818 (+/- 23.09%) ∆ = -39.24%
water::forces_ewald                             19457797 (+/- 12.98%)   18153628 (+/- 12.09%) ∆ =  -0.89%
water::forces_wolf                                873800 (+/- 23.91%)     767171 (+/- 30.83%) ∆ =  +6.92%
water::virial_ewald                             90876483 (+/-  8.42%)   83790050 (+/- 13.67%) ∆ =  +5.26%
water::virial_wolf                                854935 (+/- 26.47%)     872880 (+/- 22.46%) ∆ =  -4.01%

@Luthaf Luthaf changed the title Some benchmarks improvements [WIP] Some benchmarks improvements Apr 3, 2017
@antoinewdg
Copy link
Contributor

This is the problem when we open two PR that depend on each other, it all breaks when we merge/rebase the base one :(.

I agree that this does not look very good considering the amount of code complexity it adds.

However the part where the file name is added in the out is super nice ! Maybe we could cherry pick it and leave the rest for later ?

This will make reading benchmark output and comparison easier
@Luthaf Luthaf force-pushed the benchmarks branch 2 times, most recently from 2bc493f to 4f1a1c4 Compare April 6, 2017 10:25
@Luthaf
Copy link
Member Author

Luthaf commented Apr 6, 2017

I removed the second commit, the corresponding patch is here for posterity.

@Luthaf Luthaf changed the title [WIP] Some benchmarks improvements Some benchmarks improvements Apr 6, 2017
@antoinewdg
Copy link
Contributor

This little change will simplify my life :)

@antoinewdg antoinewdg merged commit bd44d07 into master Apr 8, 2017
@antoinewdg antoinewdg deleted the benchmarks branch April 8, 2017 16:41
@antoinewdg antoinewdg restored the benchmarks branch April 8, 2017 16:41
@Luthaf Luthaf deleted the benchmarks branch April 8, 2017 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants