Now the header is correct also in mpi run benchmark run

Iximiel commented 3 months ago

Description

Reading the code again, I discovered that shuffled and domain decomposition could be automatically changed without the user input, during the setup of the benchmark, I changed the output of the header to reflect that.

Sorry for the Friday PR, this is a small thing that I am sure to forget

Target release

I would like my code to appear in release 2.10

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.
[ ] I verified that all regtests are passed successfully on GitHub Actions.

GiovanniBussi commented 2 months ago

@Iximiel if I read the code correctly, this means that even if you run with --shuffled and without --domaindecomposition, the log will display running with --domaindecomposition. I think it is not correct. Did I misunderstand something?

Iximiel commented 2 months ago

In my intention should be the other way around, tomorrow I will check if is correct

maybe I should also add an 1 step benchmark regtest to enforce the beahviour of the header of the output

Iximiel commented 2 months ago

I checked out and compiled this branch:

mpirun -np2 plumed benchmark --natoms=2 --nsteps=1 --domain-decomposition starts with

BENCH:  Welcome to PLUMED benchmark
BENCH:  Using --kernel=this
BENCH:  Using --plumed=plumed.dat
BENCH:  Using --nsteps=1
BENCH:  Using --natoms=2
BENCH:  Using --maxtime=-1
BENCH:  Using --shuffled
BENCH:  Using --domain-decomposition
BENCH:  Using --sleep=0
BENCH:  Using --atom-distribution=line
BENCH:  Initializing the setup of the kernel(s)

plumed benchmark --natoms=2 --nsteps=1 --shuffled starts with:

BENCH:  Welcome to PLUMED benchmark
BENCH:  Using --kernel=this
BENCH:  Using --plumed=plumed.dat
BENCH:  Using --nsteps=1
BENCH:  Using --natoms=2
BENCH:  Using --maxtime=-1
BENCH:  Using --shuffled
BENCH:  Using --sleep=0
BENCH:  Using --atom-distribution=line
BENCH:  Initializing the setup of the kernel(s)

so shuffled does not activate the domain decomposition, but the dd activates shuffled, as intended

I also discovered this: plumed --no-mpi benchmark --natoms=2 --nsteps=1 --domain-decomposition echoes the first lines of the output then crashes, because it needs mpi function. But I think that the error message that appears is clear enough for an advanced user:

terminate called after throwing an instance of 'PLMD::Plumed::ExceptionError'
  what():  
(tools/Communicator.cpp:94) void PLMD::Communicator::Set_comm(const PLMD::TypesafePtr&)
+++ assertion failed: initialized()
you are trying to use an MPI function, but MPI is not initialized

GiovanniBussi commented 2 months ago

so shuffled does not activate the domain decomposition, but the dd activates shuffled, as intended

This was my point. I mean, the log should not say --shuffled if the user didn't put it on the command line the --shuffled flag. It's ok to write Atoms are shuffled. But not --shuffle was used, if it was not...

Iximiel commented 2 months ago

When I set up this, my idea was to show the actual configuration.

I added this PR on top of the old one because I noted that having shuffle show on the output immediately after parsing did not show the configuration which the benchmark was running with.

On the other hand, with this approach, we have reproducible output even in case of a change of default parameters, so I show all the parameters even if the user did not specify them in the command line.

If I do a mpirun -np2 plumed benchmark --natoms=2 --nsteps=1 I get again:

BENCH:  Welcome to PLUMED benchmark
BENCH:  Using --kernel=this
BENCH:  Using --plumed=plumed.dat
BENCH:  Using --nsteps=1
BENCH:  Using --natoms=2
BENCH:  Using --maxtime=-1
BENCH:  Using --shuffled
BENCH:  Using --domain-decomposition
BENCH:  Using --sleep=0
BENCH:  Using --atom-distribution=line
BENCH:  Initializing the setup of the kernel(s)

Because it is hardcoded to use the dd in a mpi run and to use shuffled if the dd is on. On that I probably should add a note on the number of processes used and maybe also on the threads, if avaiable.

carlocamilloni commented 1 month ago

@Iximiel @GiovanniBussi I think here the idea is to report the actual running configuration including both implicit and explicit choices, so if I would say it can be merged. I would retarget on 2.10 and report the number of process/threads as well

Iximiel commented 1 month ago

@Iximiel @GiovanniBussi I think here the idea is to report the actual running configuration including both implicit and explicit choices

Yes

plumed / plumed2