search

cosmodesi / pyrecon

package for BAO reconstruction
BSD 3-Clause "New" or "Revised" License
9 stars 7 forks source link

set nthreads dynamically #2

Closed seshnadathur closed 2 years ago

seshnadathur commented 2 years ago

In the standalone pyrecon implementation, set nthreads based on the number of cpus available (using multiprocessing.cpu_count()) instead of based on user input.

When working on slurm or other HPC systems, the user might want to request a different number of cpus for different runs by editing the job submission script, depending on resources available. This way they don't need to also edit the config yaml file to keep them in sync.

adematti commented 2 years ago

Thanks for the PR! I think one can leave nthreads blank. Then it should default to OMP_NUM_THREADS, or OpenMP's default, typically number of cores (just added a comment about that in the config file, commit 249e165733370dc26759b85c5ddfbeb0810b6cc0). Does this match your needs?

seshnadathur commented 2 years ago

I didn't realise it would automatically default to the number of cores if not set. I guess that would work – I'll try it out and report back.

adematti commented 2 years ago

Can I close this PR?

seshnadathur commented 2 years ago

So I tried this on NERSC, and it works fine provided I first export OMP_NUM_THREADS to the number of cores used (otherwise it appears to default to 1). It's a minor inconvenience but it's fine to close the PR if you want.

adematti commented 2 years ago

Thanks for trying! On my laptop the number of threads defaults to the number of cores if OMP_NUM_THREADS is not set. This is more an issue of OpenMP setting on the specific environment than pyrecon itself, which will just query the number of threads used by OpenMP, here https://github.com/cosmodesi/pyrecon/blob/4664f33889f789f1ededd7447ba750db4817e7d4/pyrecon/mesh.py#L148 from here https://github.com/cosmodesi/pyrecon/blob/4664f33889f789f1ededd7447ba750db4817e7d4/src/mesh.c#L13, so I'll close the PR.