[HDF5] Update - 1 - Refactor HDF5File #11473
Conversation
| if(USE_MPI) | ||
| target_link_libraries(KratosHDF5Core PUBLIC KratosCore KratosMPICore PRIVATE ${HDF5_C_LIBRARIES} ${KRATOS_HDF5_APPLICATION_EXTRA_LIBS}) | ||
| else(USE_MPI) | ||
| target_link_libraries(KratosHDF5Core PUBLIC KratosCore PRIVATE ${HDF5_C_LIBRARIES} ${KRATOS_HDF5_APPLICATION_EXTRA_LIBS}) | ||
| endif(USE_MPI) |
There was a problem hiding this comment.
I recommend splitting this into a separate MPI-Extension, same as the other apps
There was a problem hiding this comment.
Would it be ok if I do it in a seperate PR? I think that will be cleaner, because there are some other files which also needs to go into this mpi-extension.
applications/HDF5Application/custom_processes/hdf5_xdmf_connectivities_writer_process.cpp
Outdated
Show resolved
Hide resolved
applications/HDF5Application/custom_processes/hdf5_xdmf_connectivities_writer_process.cpp
Outdated
Show resolved
Hide resolved
| private: | ||
| ///@name Member Variables | ||
| ///@{ | ||
|
|
||
| DataCommunicator const * mpDataCommunicator; |
There was a problem hiding this comment.
I'm the one to blame here, but I'll stand by my opinion. Storing a const member always messes up move and copy semantics. In this case, copy construction/assignment and move assignment doesn't make sense, but I don't see why move construction should be forbidden.
| @@ -22,67 +49,9 @@ bool IsPath(const std::string& rPath) | |||
| return regex_match(rPath, std::regex("(/[\\w\\(\\)]+)+")); | |||
There was a problem hiding this comment.
perhaps this can be done better with std::filesystem
There was a problem hiding this comment.
I am not sure how to do this with std::filesystem since these paths are not actual file paths, but hdf5 paths within one hdf5 file.
applications/HDF5Application/custom_utilities/data_type_utilities.h
Outdated
Show resolved
Hide resolved
|
@matekelemen @philbucher I applied your suggestions. Could you have a look? Thanks :D |
sunethwarna
changed the title
|
LGTM |
|
@matekelemen I changed to |
📝 Description
This PR refactors
HDF5File. Followings changes/updates were applied.HDF5Filector now requiresDataCommunicator. The python ctor still has a ctor withoutDataCommunicator, in this case a warning is thrown. This is planned to be removed once the python side of theHDF5Applicationis updated.mpicalls are now done viaDataCommunicator.HDF5FileSerialandHDF5FileParallelare removed. Everything is done now inHDF5Filearray_1d<double, 4>,array_1d<double, 6>,array_1d<double, 9>HDF5FileParallelversion throws an error if the dataset is existing when writing, butHDF5SerialFileretireves the existing and changes it. The behaviour is now uniform in mpi and openmp.HDFFileupdated.Requires #11469
This closes #10728
🆕 Changelog