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: Handle unknown datatypes in datasets #1469

Merged
merged 5 commits into from
Aug 8, 2023

Conversation

franzpoeschel
Copy link
Contributor

@franzpoeschel franzpoeschel commented Jun 30, 2023

First commit: Throw error::ReadError in HDF5IOHandlerImpl::openDataset(). Upon encountering an error, the middle-end will know that something has gone wrong that it can then recover from, skipping the dataset.

Second commit: Sometimes, datasets use custom datatypes based on a native type. This can be supported by checking the parent datatypes if the actual datatypes is not recognized. See here for an example that uses enums to emulate booleans.

@ax3l
Copy link
Member

ax3l commented Jun 30, 2023

Thanks for the patch!

This seems to fail on 32bit Windows right now @franzpoeschel

@ax3l
Copy link
Member

ax3l commented Jun 30, 2023

Does this need test coverage? :)

@ax3l ax3l modified the milestones: 0.15.2, 0.15.3 Jun 30, 2023
@franzpoeschel
Copy link
Contributor Author

franzpoeschel commented Jul 3, 2023

The Windows tests look like a random failure. EDIT: Yep, after restarting the CI, it runs fine.
Testing this is a bit difficult since the datasets that are fixed by this cannot be created by the openPMD-api, so we would need to first manually create one and then add it to the sample datasets. Might still be worth it as we could use this to ensure read compatibility with old PIConGPU HDF5 files which we apparently did not so far.

@franzpoeschel
Copy link
Contributor Author

For testing, we'll need openPMD/openPMD-example-datasets#20

test/SerialIOTest.cpp Fixed Show fixed Hide fixed
};
} // namespace

TEST_CASE("git_hdf5_legacy_picongpu", "[serial][hdf5]")

Check notice

Code scanning / CodeQL

Unused static function Note test

Static function C_A_T_C_H_T_E_S_T_50 is unreachable (
autoRegistrar51
must be removed at the same time)
@ax3l ax3l modified the milestones: 0.15.3, 0.15.2 Jul 25, 2023
Copy link
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

🎉

@ax3l ax3l merged commit 3bc460d into openPMD:dev Aug 8, 2023
28 checks passed
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.

2 participants