Closed lepperdinger closed 1 year ago
Hi @lepperdinger, this looks great! Thank you a lot for contributing to HERMES. As I don't see any changes in your PR to the base code and functionality, besides the CMake/header files - I don't see also any reason for extensive testing from our side before merging the PR, i.e., if this addition works for you - that's good enough IMHO. The only thing left is to check that the compilation process does not fail on various platforms with these changes to the CMake/header files.
I set the option ENABLE_SYS_HDF5
to OFF
in CMakeLists.txt
. The HDF5 library and also cosmicrays::Picard3D
is now by default deactivated. It can be activated via the CMake flag -DENABLE_SYS_HDF5=ON
if needed. That way, not everyone is forced to install the HDF5 library.
Hi @lepperdinger,
I've tried to test your PR but there is something missing:
unknown file: Failure C++ exception with description "filesystem error: directory iterator cannot open directory: No such file or directory [ (...)/.virtualenvs/hermes/share/hermes/data/CosmicRays/Picard_testing]" thrown in the test body
where can I download it and include it in our data bundle?
Also, I would automatically enable the Picard3D feature if the HDF5 library is found and automatically disable if it is not. By doing that we can also automatically test it with our CI, e.g., GitHub Actions.
In order to make the tests work, the corresponding test files (i.e., cosmic ray fluxes by PICARD) would have to be added to
https://heat.gssi.it/hermes/data/hermes-data.tar.gz
.
Hi @lepperdinger, thanks a lot for this contribution! Using HDF5 file will be highly beneficial for this code. I will add it to the hermes-data file.
Hi @lepperdinger,
I've tried to test your PR but there is something missing: (...)
Ups, sorry, I missed the last part of your PR until I read @carmeloevoli's comment.
Hi @lepperdinger, I've tried to test your PR but there is something missing: (...)
Ups, sorry, I missed the last part of your PR until I read @carmeloevoli's comment.
Hi @adundovi, nevertheless... I cannot find the file
CosmicRays/Picard_testing/Picard_testing_tfinal/Hydrogen_1.h5
is it uploaded somewhere or should I get form @lepperdinger ?
Hi @adundovi and @carmeloevoli,
I now uploaded the test files to https://github.com/lepperdinger/picard_test_fluxes/. For the tests, the directory Picard_testing
has to be added to the CosmicRays
directory: data/CosmicRays/Picard_testing
. The fluxes of the test files have a very low resolution to keep the size of the test files small.
Also, I would automatically enable the Picard3D feature if the HDF5 library is found and automatically disable if it is not. By doing that we can also automatically test it with our CI, e.g., GitHub Actions.
It now automatically enables Picard3D
when CMake finds the HDF5 library.
Also, I would automatically enable the Picard3D feature if the HDF5 library is found and automatically disable if it is not. By doing that we can also automatically test it with our CI, e.g., GitHub Actions.
It now automatically enables
Picard3D
when CMake finds the HDF5 library.
Hi, great work! I've updated the data bundle with your test files. However, I've changed slightly the path to be consistent with other data folders, e.g., we use CamelCase instead of snake_case and if the extension is named Picard3D, this should also be contained in the folder name. Check if this is OK with you, or there are some other constraints I'm unaware of? If this is OK, please update the code.
#define DEFAULT_CR_DIRECTORY "CosmicRays/Picard3DTest"
Also, I would automatically enable the Picard3D feature if the HDF5 library is found and automatically disable if it is not. By doing that we can also automatically test it with our CI, e.g., GitHub Actions.
It now automatically enables
Picard3D
when CMake finds the HDF5 library.Hi, great work! I've updated the data bundle with your test files. However, I've changed slightly the path to be consistent with other data folders, e.g., we use CamelCase instead of snake_case and if the extension is named Picard3D, this should also be contained in the folder name. Check if this is OK with you, or there are some other constraints I'm unaware of? If this is OK, please update the code.
#define DEFAULT_CR_DIRECTORY "CosmicRays/Picard3DTest"
Hi, sure! I adapted the code to the new file path.
Great! I've updated pybind11 and added hdf5 to github actions to have CI for Picard3D. And finally, your PR is merged! Thank you for your contribution.
I implemented the class
hermes::cosmicrays::Picard3D
with which HERMES can use the cosmic ray fluxes simulated by PICARD.Since PICARD saves its simulation results in HDF5 files, I also added the class
Hdf5Reader
and added the HDF5 library toCMakeLists.txt
. On Debian or on Debian based distributions (e.g., Ubuntu, Linux Mint) the HDF5 library usually can be installed viasudo apt install libhdf5-dev
. On Fedora it can be installed viasudo dnf install hdf5-devel
.I additionally changed the C++ standard in CMakeLists.txt from C++14 to C++17 to be able to use the C++17 feature
std::filesystem
.In order to make the tests work, the corresponding test files (i.e., cosmic ray fluxes by PICARD) would have to be added to
https://heat.gssi.it/hermes/data/hermes-data.tar.gz
.