Closed junverferth closed 9 months ago
I'm all for adding this as a config variable. Would it be possible to add it to HYDRAD/source/config.h
instead? Currently, pydrad
only writes to the various config.h
and .cfg
files.
It should be added to the HYDRAD/source/config.h file along with the directive which defines the number of threads to use. At some point, I may redo the parallelization code to remove the need to specify a CHUNK_SIZE, I’m not sure that it makes an awful lot of difference (although I also haven’t experimented much with it either).
From: Will Barnes @.> Sent: Friday, March 25, 2022 10:45 AM To: rice-solar-physics/pydrad @.> Cc: Subscribed @.***> Subject: Re: [rice-solar-physics/pydrad] OpenMP support (Issue #152)
I'm all for adding this as a config variable. Would it be possible to add it to HYDRAD/source/config.h instead? Currently, pydrad only writes to the various config.h and .cfg files.
— Reply to this email directly, view it on GitHub https://github.com/rice-solar-physics/pydrad/issues/152#issuecomment-1079155717 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ACC6C7SLVL26SLTBCNNGFY3VBXNQJANCNFSM5RUD2YUA . You are receiving this because you are subscribed to this thread. https://github.com/notifications/beacon/ACC6C7VNKQADU4RUT3URFRDVBXNQJA5CNFSM5RUD2YUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOIBJJYBI.gif Message ID: @. @.> >
At least in my version, adding it to config.h did not fix the problem but adding to eqns.h does.
It does still leave the issue as mentioned above about thread count, since "set OMP_NUM_THREADS=
Huh that seems odd. Looking at eqns.h
, it includes cell.h
which includes params.h
which then includes config.h
so shouldn't it be equivalent to sticking it in eqns.h
?
The problem with providing it as a configurable parameter and then sticking it inside eqns.h
is that eqns.h
includes a lot of other code that is not auto-generated as opposed to the config.h
files which all include directives set based on user input (or are dependent on it).
It does still leave the issue as mentioned above about thread count, since "set OMP_NUM_THREADS=" doesn't work and instead utilizes all cores regardless.
Is this a pydrad
or HYDRAD
issue? If you set OMP_NUM_THREADS
through the GUI, does it work there?
It does still leave the issue as mentioned above about thread count, since "set OMP_NUM_THREADS=" doesn't work and instead utilizes all cores regardless.
Is this a pydrad or HYDRAD issue? If you set OMP_NUM_THREADS through the GUI, does it work there?
It works through the GUI on my local windows machine. However, on a remote linux machine without the GUI it fails, and there's no handle for me to set that in pydrad
Huh that seems odd. Looking at eqns.h, it includes cell.h which includes params.h which then includes config.h so shouldn't it be equivalent to sticking it in eqns.h?
The problem with providing it as a configurable parameter and then sticking it inside eqns.h is that eqns.h includes a lot of other code that is not auto-generated as opposed to the config.h files which all include directives set based on user input (or are dependent on it).
I agree that writing to less files is better, I'm not sure why including it higher up the hierarchy doesn't work. The plot thickens as well, placing it in params.h and cell.h works.
Most odd… I’ve not encountered this before, but then I almost never manually configure HYDRAD and usually run it via the GUI.
Still, glad there is a workaround for the moment. Thanks for flagging it.
From: junverferth @.> Sent: Friday, March 25, 2022 2:22 PM To: rice-solar-physics/pydrad @.> Cc: sjbradshaw @.>; Comment @.> Subject: Re: [rice-solar-physics/pydrad] OpenMP support (Issue #152)
It does still leave the issue as mentioned above about thread count, since "set OMP_NUM_THREADS=" doesn't work and instead utilizes all cores regardless.
Is this a pydrad or HYDRAD issue? If you set OMP_NUM_THREADS through the GUI, does it work there?
It works through the GUI on my local windows machine. However, on a remote linux machine without the GUI it fails, and there's no handle for me to set that in pydrad
Huh that seems odd. Looking at eqns.h, it includes cell.h which includes params.h which then includes config.h so shouldn't it be equivalent to sticking it in eqns.h?
The problem with providing it as a configurable parameter and then sticking it inside eqns.h is that eqns.h includes a lot of other code that is not auto-generated as opposed to the config.h files which all include directives set based on user input (or are dependent on it).
I agree that writing to less files is better, I'm not sure why including it higher up the hierarchy doesn't work. The plot thickens as well, placing it in params.h and cell.h works.
— Reply to this email directly, view it on GitHub https://github.com/rice-solar-physics/pydrad/issues/152#issuecomment-1079362771 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ACC6C7R3G6WWM4IWXFDSLTLVBYG3VANCNFSM5RUD2YUA . You are receiving this because you commented. https://github.com/notifications/beacon/ACC6C7WQQQ7ZRD6Y4PJDWJTVBYG3VA5CNFSM5RUD2YUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOIBK4JUY.gif Message ID: @. @.> >
I agree that writing to less files is better, I'm not sure why including it higher up the hierarchy doesn't work. The plot thickens as well, placing it in params.h and cell.h works.
This is maybe a silly question but are you by chance running the pydrad configuration step after you edit the config.h
file? If so, pydrad would be overwriting your changes to that file. The others (params, cell, eqns) are not modified by pydrad so changes there would still persist.
It is not a silly question, Because that's exactly what I was doing, modifying the .h files in the source directory. But with pydrad remakingconfig.h
then that change would be erased. That solves the mystery of why config.h
wasn't working I believe.
Well I'm kind of relieved that that is the case!
Ok, in that case, I would recommend modifying the hydrad.config.h
template by adding the following lines in the // **** Solver ****
section,
{% if general.openmp_chunk_size is defined -%}
#define OPENMP
#define CHUNK_SIZE {{ general.openmp_chunk_size }}
{%- endif %}
The way I understand it (correct me if I'm wrong), you can't use OpenMP without specifying chunk size so having both the general.use_openmp
and openmp_chunk_size
options is redundant so we can just use the chunk size parameter to signal that OpenMP should be used.
re: OMP_NUM_THREADS
where should that be set? Where does the GUI set it?
The GUI places it in a batch file that calls HYDRAD.exe
cd HYDRAD/build_scripts
call build_HYDRAD_OPENMP.bat
cd ../..
set OMP_NUM_THREADS=4
HYDRAD.exe
OMP_NUM_THREADS
is a sys variable for openmp at run time, so unless pydrad is going to launch the simulation i think the best it could do is to make a similar batch file that contains the last two lines.
set OMP_NUM_THREADS=4
HYDRAD.exe
Though in my experience this variable is only working on windows not linux.
Ok since it is an environment variable, I am inclined to not include that in pydrad and leave that up to the user to set. Presumably this is easy enough to set in your .bashrc
?
Re: your linux versus Windows problem, I think you'll need to use export OMP_NUM_THREADS=4
, at least for a bash shell.
OMP_NUM_THREADS is an environment variable that is set on the command line before calling the HYDRAD executable. E.g. under MS Windows one would enter: set OMP_NUM_THREADS=4 (to request 4 threads for the executable).
The GUI writes a batch/script which compiles the code, sets the number of threads, and then calls HYDRAD.exe. E.g.
cd HYDRAD/build_scripts
call build_HYDRAD_OPENMP.bat
cd ../..
set OMP_NUM_THREADS=4
HYDRAD.exe
From: Will Barnes @.> Sent: Monday, March 28, 2022 8:51 AM To: rice-solar-physics/pydrad @.> Cc: sjbradshaw @.>; Comment @.> Subject: Re: [rice-solar-physics/pydrad] OpenMP support (Issue #152)
re: OMP_NUM_THREADS where should that be set? Where does the GUI set it?
— Reply to this email directly, view it on GitHub https://github.com/rice-solar-physics/pydrad/issues/152#issuecomment-1080678115 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ACC6C7SQDRPLWKUHQQWUQGDVCG2K5ANCNFSM5RUD2YUA . You are receiving this because you commented. https://github.com/notifications/beacon/ACC6C7VK4TCO7TEV6WN5KA3VCG2K5A5CNFSM5RUD2YUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOIBU5NYY.gif Message ID: @. @.> >
Ok that's good to know. pydrad
does already call build_HYDRAD_OPENMP.bat
when compiling HYDRAD if the OpenMP option is selected.
The batch file that the GUI calls with the the threads defined is not HYDRAD/build_scripts/build_HYDRAD_OPENMP.bat
its instead HYDRAD.bat
in the base HYDRAD directory. I believe it is made by the GUI when you set the checkbox for openMP.
The batch file that the GUI calls with the the threads defined is not
HYDRAD/build_scripts/build_HYDRAD_OPENMP.bat
its insteadHYDRAD.bat
in the base HYDRAD directory. I believe it is made by the GUI when you set the checkbox for openMP.
Yep, sorry I think there was some confusion. I just meant that pydrad
does the compilation step correctly in the OpenMP case by selecting the OpenMP .bat file. The Configure
object does not provide any entrypoint for actually running HYDRAD (outside of the initial conditions). This is by design as there's really not much reason to keep a Python kernel running for as long as HYDRAD is running given that the runtimes can be long for some types of simulations.
Because setting OMP_NUM_THREADS
is only used at runtime, I'm not inclined to include it the pydrad configuration setup.
Attempting to setup a simulation using OpenMP Output looks like: INFO:
Calculating initial hydrostatic conditions...
Peak heating range = 4.3652e-004 -> 4.4668e-004 erg cm^-3 s^-1
Optimum peak heating rate = 4.4608e-004 erg cm^-3 s^-1
Writing initial conditions file...
Done!
[pydrad.configure.util] build_HYDRAD_OPENMP.bat INFO: C:\Users\JOHNUN~1\AppData\Local\Temp\tmprqt_o_ta\HYDRAD\build_scripts>g++ -O3 -flto -fopenmp -Wall -Wno-unused-variable ../source/main.cpp ../source/cell.cpp ../source/mesh.cpp ../source/eqns.cpp ../../Kinetic_Model/source/kinetic.cpp ../../Kinetic_Model/source/gamma.cpp ../../Heating_Model/source/heat.cpp ../../Radiation_Model/source/ionfrac.cpp ../../Radiation_Model/source/element.cpp ../../Radiation_Model/source/radiation.cpp ../../Radiation_Model/source/OpticallyThick/OpticallyThickIon.cpp ../../Radiation_Model/source/OpticallyThick/RadiativeRates.cpp ../../Resources/source/gammabeta.cpp ../../Resources/source/fitpoly.cpp ../../Resources/Utils/generatePieceWiseFit/source/piecewisefit.cpp ../../Resources/Utils/regPoly/regpoly.cpp ../../Resources/Utils/regPoly/nrutil.cpp ../../Resources/source/file.cpp -o ../../HYDRAD.exe [pydrad.configure.util] WARNING: ../source/eqns.cpp: In member function 'void CEquations::CalculatePhysicalQuantities()': ../source/eqns.cpp:938:35: error: 'CHUNK_SIZE' was not declared in this scope
pragma omp for schedule(dynamic, CHUNK_SIZE) \
../source/eqns.cpp: In member function 'void CEquations::EvaluateTerms(double, double*, int)': ../source/eqns.cpp:2200:35: error: 'CHUNK_SIZE' was not declared in this scope
pragma omp for schedule(dynamic, CHUNK_SIZE) \
[pydrad.configure.util]
Manually placing "#define CHUNK_SIZE 10" into eqns.h in the HYDRAD source dir works, but having a configurable entry alongside config['general']['use_openmp'] would be preferable.
Issue occurs both in linux environment and through Jupyter notebook on windows.