search

rice-solar-physics / pydrad

Python tools for setting up HYDRAD runs and parsing output
https://pydrad.readthedocs.io
MIT License
4 stars 3 forks source link

Setting `use_openmp` to True results in compilation error because of missing `CHUNK_SIZE` #163

Closed wtbarnes closed 1 year ago

wtbarnes commented 1 year ago

When setting use_openmp to True, I get the following error during the compilation step:

../source/eqns.cpp:981:35: error: use of undeclared identifier 'CHUNK_SIZE'
  981 | #pragma omp for schedule(dynamic, CHUNK_SIZE)           \
      |                                   ^
../source/eqns.cpp:2241:35: error: use of undeclared identifier 'CHUNK_SIZE'
 2241 | #pragma omp for schedule(dynamic, CHUNK_SIZE)                                           \
      |                                   ^
2 errors generated.

Presumably, this is just because I'm not setting CHUNK_SIZE as a variable anywhere. This should be changed to be a required variable if use_openmp is True. It would also be nice if this is not set to just use a sensible default option. However, I'm not sure what the best default option is. I'm assuming this variable is basically the same as OMP_NUM_THREADS?

sjbradshaw commented 1 year ago

The HYDRAD GUI defaults to OMP_NUM_THREADS=4 and CHUNK_SIZE=10. The latter can be thought of as the number of grid cells per thread; I guess it helps to tune the load-balancing and should be experimented with to find the best value on different systems. To be honest, I don’t think it’s absolutely necessary; I don’t use it in other code that I have parallelized. At some point, I will probably just get rid of it.

From: Will Barnes @.> Sent: Tuesday, November 7, 2023 3:46 PM To: rice-solar-physics/pydrad @.> Cc: Subscribed @.***> Subject: [rice-solar-physics/pydrad] Setting use_openmp to True results in compilation error because of missing CHUNK_SIZE (Issue #163)

When setting use_openmp to True, I get the following error during the compilation step:

../source/eqns.cpp:981:35: error: use of undeclared identifier 'CHUNK_SIZE' 981 | #pragma omp for schedule(dynamic, CHUNK_SIZE) \ | ^ ../source/eqns.cpp:2241:35: error: use of undeclared identifier 'CHUNK_SIZE' 2241 | #pragma omp for schedule(dynamic, CHUNK_SIZE) \ | ^ 2 errors generated.

Presumably, this is just because I'm not setting CHUNK_SIZE as a variable anywhere. This should be changed to be a required variable if use_openmp is True. It would also be nice if this is not set to just use a sensible default option. However, I'm not sure what the best default option is. I'm assuming this variable is basically the same as OMP_NUM_THREADS?

— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https:/github.com/rice-solar-physics/pydrad/issues/163__;!!BuQPrrmRaQ!hmZYC6rEjj96Kep6-Nx4EdnJago2Bcq18EFwt9AV7kl4KzHR7pNo_5x0oOL8OlRavFklgr58bkawkvBtrwvU7EnpgV5ZfspELoQ$ , or unsubscribe https://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/ACC6C7SLSYTSVGNTDOJR7WTYDKTZVAVCNFSM6AAAAAA7B4AHA2VHI2DSMVQWIX3LMV43ASLTON2WKOZRHE4DEMRYGU4DCMQ__;!!BuQPrrmRaQ!hmZYC6rEjj96Kep6-Nx4EdnJago2Bcq18EFwt9AV7kl4KzHR7pNo_5x0oOL8OlRavFklgr58bkawkvBtrwvU7EnpgV5ZSGDx3hM$ . You are receiving this because you are subscribed to this thread. https://github.com/notifications/beacon/ACC6C7TLHCKOIEHK47OTMFDYDKTZVA5CNFSM6AAAAAA7B4AHA2WGG33NNVSW45C7OR4XAZNFJFZXG5LFVJRW63LNMVXHIX3JMTHHMJ2H6Q.gif Message ID: @. @.> >

wtbarnes commented 1 year ago

Ok thanks that's good to know. I've gone ahead and added it as an additional parameter for now. We can always remove it later if HYDRAD does not support that option in the future.