Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HDF5: Delete and re-create attribute when overwriting with diff. type #1697

Merged

Conversation

franzpoeschel
Copy link
Contributor

@franzpoeschel franzpoeschel commented Nov 19, 2024

Seen in a file that was originally create with h5py and the opened in R/W-mode with openPMD-api. Since h5py and openPMD-api apparently use different vec-string types, this leads to corrupted files.
Since we do support attributes with multiple possible types, the type of an existing attribute should be checked before reuse.

TODO:

  • Test

@franzpoeschel franzpoeschel added this to the 0.16.2 milestone Nov 19, 2024
@franzpoeschel franzpoeschel force-pushed the fix-hdf5-overwrite-attribute-type branch from 6f937fb to f33f078 Compare November 19, 2024 17:05
@franzpoeschel franzpoeschel force-pushed the fix-hdf5-overwrite-attribute-type branch 2 times, most recently from 8dad8db to 79f752d Compare December 4, 2024 14:37
@@ -7522,6 +7522,8 @@ void groupbased_read_write(std::string const &ext)

E_x.setAttribute("updated_in_run", 0);
E_y.setAttribute("updated_in_run", 0);
E_y.setAttribute("changed_datatype_in_run", 0);
write.close();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.close() calls added for stricter error check, the destructors will swallow exceptions.

@franzpoeschel franzpoeschel enabled auto-merge (squash) December 4, 2024 14:38
@franzpoeschel franzpoeschel force-pushed the fix-hdf5-overwrite-attribute-type branch from 79f752d to 6ee8426 Compare December 4, 2024 16:33
@franzpoeschel franzpoeschel merged commit 77a55a3 into openPMD:dev Dec 4, 2024
31 checks passed
@franzpoeschel franzpoeschel modified the milestones: 0.16.2, 0.16.1 Dec 20, 2024
franzpoeschel added a commit to ax3l/openPMD-api that referenced this pull request Dec 20, 2024
…openPMD#1697)

* HDF5: Delete and re-create attribute when overwriting with diff. type

* WIP: Change datatype in test

Somehow truncation test is broken by this

* Add forgotten H5Aclose() call
ax3l added a commit that referenced this pull request Jan 16, 2025
* Release: 0.16.1

Signed-off-by: Axel Huebl <axel.huebl@plasma.ninja>

* Fix new_version.py

* Update version in windows.yml

* Reformat with yapf3

* Fix greedy Regex parsing

* Changelog: Add PRs #1681 #1679 #1678

* Fix: CMake Internal Control (#1678)

The `openPMD_USE_INTERNAL_*` options are for general internal/external
control. They were accidentally overwritten by new tar flags
and sometimes incomplete.

Signed-off-by: Axel Huebl <axel.huebl@plasma.ninja>

* Revert "CMake Tar: TOML11 3.7.1 (#1673)" (#1679)

This reverts commit aea4b72.

* Replace deprecated makeSuite with loadTestsFromTestCase (#1681)

Removed with Python 3.13

* Doc: Fix Shipped Internally (#1682)

Downloaded now.

Signed-off-by: Axel Huebl <axel.huebl@plasma.ninja>

* `PYBIND11_FINDPYTHON=ON` (#1684)

Reuse our `find_package(Python ...)` call and use new CMake logic in
pybind11.
https://pybind11.readthedocs.io/en/stable/compiling.html#modules-with-cmake
https://cmake.org/cmake/help/latest/command/find_package.html#config-mode-version-selection

Signed-off-by: Axel Huebl <axel.huebl@plasma.ninja>

* Fix ICX build (#1690)

Error was:
 /home/runner/work/openPMD-api/openPMD-api/include/openPMD/backend/Container.hpp:262:32:
error: no member named 'm_container' in 'Container<T, T_key,
T_container>'
  262 |         container().swap(other.m_container);

* Fix Leftover ADIOS1 Mentions (#1683)

* Fix Leftover ADIOS1 Mentions

Signed-off-by: Axel Huebl <axel.huebl@plasma.ninja>

* Remove ADIOS1 from examples

---------

Signed-off-by: Axel Huebl <axel.huebl@plasma.ninja>
Co-authored-by: Franz Pöschel <franz.poeschel@gmail.com>

* Explicitly specify ADIOS2 components in openPMDConfig.cmake (#1693)

* Explicitly specify ADIOS2 components in openPMDConfig.cmake

When including openPMD in downstream CMake and that CMake script
contains Fortran, ADIOS2 will automatically detect that. But since we
do not reexport the ADIOS2 headers, we only need the components that
openPMD explicitly uses: CXX and MPI.

* Same in CMakeLists.txt

* Remove mpirun_workaround.sh (#1698)

Seems to no longer be necessary in the affected CI runs

* Properly check for empty HDF5_VERSION in CMake (#1702)

The old test apparently invoked some weird CMake legacy behavior.
This should make the pinned HDF5 Conda version unnecessary (#1701) since
the version with missing HDF5_VERSION is correctly dealt with.

* HDF5: Delete and re-create attribute when overwriting with diff. type (#1697)

* HDF5: Delete and re-create attribute when overwriting with diff. type

* WIP: Change datatype in test

Somehow truncation test is broken by this

* Add forgotten H5Aclose() call

* TOUCH IOTask: Avoid setting files as dirty in non-write modes (#1704)

* Bugfix: Consider dirty upon touch() only in write modes

* Add error check to avoid writing in read-only mode

* Changelog

* Use ULONG for particle patches

* Upgrade appleclang14 build to MacOS 13

> The macOS-12 environment is deprecated, consider switching to macOS-13,
macOS-14 (macos-latest) or macOS-15. For more details, see
actions/runner-images#10721

* Update Changelog

* Release Date

---------

Signed-off-by: Axel Huebl <axel.huebl@plasma.ninja>
Co-authored-by: Franz Pöschel <franz.poeschel@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant