ec-jrc / pyPoseidon

Framework for Hydrodynamic simulations
https://pyposeidon.readthedocs.io/
European Union Public License 1.2
20 stars 8 forks source link

mpi: Only use `--use-hwthreads-cpu` if the MPI backend is OpenMPI #43

Closed pmav99 closed 2 years ago

pmav99 commented 2 years ago

mpich doesn't support the --use-hwthreads-cpu flag, so we need to only apply it when the MPI backend is OpenMPI

Continuation of: f05ef2e1 and #34

Fixes #41

brey commented 2 years ago

I think the user still can not define use_threads. Maybe pass it along the call in schism.py ?

pmav99 commented 2 years ago

Yes this is not exposed to the user, I didn't want to mess with the user facing API and i was trying to keep things compatible with the current behavior. Since there is already an option for the number of CPUs, i.e. ncores we might need to think how to expose this to the users.

As things are now:

To make it more concrete, if the user has a 4/8 machine and since use_threads is always True, the rules should go like this:

ncores = None + OpenMPI --> mpirun --use-hwthreads-cpu -n 8
ncores = None + mpich   --> mpirun -n 8
ncores = 1 + OpenMPI    --> mpirun --use-hwthreads-cpu -n 1 
ncores = 4 + OpenMPI    --> mpirun --use-hwthreads-cpu -n 4
ncores = 8 + OpenMPI    --> mpirun --use-hwthreads-cpu -n 8
ncores = 1 + mpich      --> mpirun -n 1
ncores = 4 + mpich      --> mpirun -n 4
ncores = 8 + mpich      --> mpirun -n 8

The default value of --use-threads should be changed to False if we run benchmarks and we conclude that hyper-threading is actually bad for schism. In that case we will need to revisit this

brey commented 2 years ago

Makes sense