Emancipation from serialbox via HDF5 for C-style variants #62
Conversation
…), e.g., cloudsc_c
There was a problem hiding this comment.
This looks pleasantly straightforward. Sorry for taking so long to look at this.
I've pointed out how to change the CMake integration, which should resolve your problems. Otherwise (I haven't flagged this in code), there are a few leftover commented code snippets which should probably be removed.
src/cloudsc_c/CMakeLists.txt
Outdated
| $<INSTALL_INTERFACE:include> | ||
| $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/cloudsc> | ||
| PUBLIC_LIBS | ||
| Serialbox::Serialbox_C | ||
| $<${HAVE_OMP}:OpenMP::OpenMP_C> | ||
| $<${HAVE_HDF5}:hdf5> |
There was a problem hiding this comment.
You should use here the target exported via FindHDF5:
| $<${HAVE_HDF5}:hdf5> | |
| $<${HAVE_HDF5}:hdf5::hdf5> |
This will automatically inject include paths, link flags & co, so you can drop all of the PRIVATE_INCLUDES part and link_directories fuff.
|
Additional remark: Since this will allow to always build the C variant, the dwarf-p-cloudsc/.github/scripts/verify-targets.sh Lines 11 to 16 in 60825ea |
…es not longer depend on serialbox
|
@reuterbal Thanks for pointing out how to change the CMake integration! |
|
I'd say we can leave this until the overall refactoring to avoid doing the same work twice. Do you want to apply the same changes to the C-based kernel language variants, too, or will this be separate PRs? |
|
I'll make it a separate PR. |
🥳
Draft/POC implementation HDF5 input and reference reading for C-style variants to not rely on serialbox for those variants.
Before I am tempted to over-engineer this I thought I'd make a Draft PR to allow for some comments/feedback and to allow a discussion about a potential restructuring/refactoring of the C-style variants.
Tested on
Obviously more testing necessary before thinking about merging this.
Some hacks in the CMakeLists.txt due to problems with HDF5 on AC.