Make the System Sync + Send

[Luthaf]

To make the System Send + Sync I applied the following changes:

1. the GlobalPotential trait now require Send + Sync;
2. methods of both GlobalPotential and GlobalCache take self by non-mutable reference;

This means that the System no longer has special knowledge of the global potentials and the RefCell are gone in Interactions.

This also means that we can not implement CoulombicPotential for Ewald right away, but we need to go through struct SharedEwald(RwLock<Ewald>) to get both interior mutability and Send + Sync.

@antoinewdg, in #131 you used Arc<Mutex<_>> for this, why the Arc?

[Luthaf]

And here are the benchmarks!

$ cargo benchcmp --threshold 2 740eee07.log 5e70f33a.log
 name                                  740eee07.log ns/iter  5e70f33a.log ns/iter  diff ns/iter   diff %
 cache_move_all_rigid_molecules        1,119,339             1,078,509                  -40,830   -3.65%
 cache_move_all_rigid_molecules_ewald  8,303,096             7,891,018                 -412,078   -4.96%
 cache_move_particle                   191,847               183,088                     -8,759   -4.57%
 cache_move_particle_ewald             355,302               297,836                    -57,466  -16.17%
 cache_move_particle_wolf              98,163                87,180                     -10,983  -11.19%
 cache_move_particles_ewald            797,484               699,001                    -98,483  -12.35%
 cache_move_particles_wolf             453,070               419,317                    -33,753   -7.45%
 energy                                2,318,900             1,991,234                 -327,666  -14.13%
 energy                                1,073,797             1,097,253                   23,456    2.18%
 energy_ewald                          603,803               502,722                   -101,081  -16.74%
 energy_ewald                          933,875               880,945                    -52,930   -5.67%
 energy_wolf                           468,271               399,198                    -69,073  -14.75%
 energy_wolf                           614,831               648,617                     33,786    5.50%
 forces                                2,716,057             2,252,561                 -463,496  -17.07%
 forces_ewald                          14,724,435            11,603,885              -3,120,550  -21.19%
 forces_ewald                          21,202,124            18,542,706              -2,659,418  -12.54%
 forces_wolf                           875,923               813,276                    -62,647   -7.15%
 virial                                3,013,842             2,642,224                 -371,618  -12.33%
 virial                                1,562,973             1,390,100                 -172,873  -11.06%
 virial_ewald                          86,752,276            83,053,938              -3,698,338   -4.26%
 virial_wolf                           572,575               555,970                    -16,605   -2.90%
 virial_wolf                           865,106               845,681                    -19,425   -2.25%

I don't get why there is a win in all cases, maybe the optimizer doing different stuff with the &self methods? The results are reproducible across multiple benchmark runs.

[antoinewdg]

@antoinewdg, in #131 you used Arc<Mutex<_>> for this, why the Arc?

As I told you my PR was not really meant to be merged, I did not really think about it :). Basically it was because the System needs to be Clone because of the Resize MC move (which is a little weird, but would be solved by separating the system configuration from the interactions).

I'll run the benchmarks on my machine, the difference seems weird : all these changes are not supposed to affect any performance critical part of the code.

[antoinewdg]

The results are more moderate on my machine:

$ cargo benchcmp no_sync sync --threshold 2
 name                        no_sync ns/iter  sync ns/iter  diff ns/iter  diff % 
 cache_move_particle_ewald   432,798          422,830             -9,968  -2.30% 
 cache_move_particles        352,396          341,654            -10,742  -3.05% 
 cache_move_particles_ewald  899,781          880,075            -19,706  -2.19% 
 energy_ewald                888,770          864,098            -24,672  -2.78% 
 energy_wolf                 498,392          474,542            -23,850  -4.79% 
 forces_ewald                16,116,000       15,305,386        -810,614  -5.03% 
 forces_ewald                23,518,272       22,574,122        -944,150  -4.01% 
 virial_ewald                69,958,093       71,428,725       1,470,632   2.10% 

On my side I'm OK to merge, you'll just need to resolve the merge conflicts.

[Luthaf]

Rebased!

Basically it was because the System needs to be Clone

OK. I went with the 'manually implement Clone' solution. It might be a bit more maintenance work, but this code is expected to go away anyway.

I'll run the benchmarks on my machine, the difference seems weird : all these changes are not supposed to affect any performance critical part of the code.

Yes, there should not be any difference. The only explanation I see would be the optimizer kicking in at some place.