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

Don't require unitSI when reading a patch record component #1470

Merged

Conversation

franzpoeschel
Copy link
Contributor

The openPMD standard is not quite clear if unitSI is required for patch record components. At least for numParticles and numParticlesOffset, they don't really make sense.
The openPMD-api currently requires that these attributes are set and will fail at reading time if they are not (and subsequently skip the components).

With this PR, the openPMD-api ignores if they are not present, but still writes them to maintain compatibility with versions of the openPMD-api that are not so lenient.

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.

Thanks! LGTM!

@ax3l ax3l added the internal label Jun 30, 2023
@ax3l ax3l merged commit c09d9bc into openPMD:dev Jun 30, 2023
@ax3l ax3l self-assigned this Jun 30, 2023
franzpoeschel added a commit to franzpoeschel/openPMD-api that referenced this pull request Jul 24, 2023
Changes there left the PatchRecordComponent dirty after parsing
ax3l pushed a commit that referenced this pull request Jul 25, 2023
Changes there left the PatchRecordComponent dirty after parsing
ax3l pushed a commit to ax3l/openPMD-api that referenced this pull request Aug 16, 2023
Changes there left the PatchRecordComponent dirty after parsing
ax3l added a commit that referenced this pull request Aug 19, 2023
* Fix gcc9 warning

the implicitly-defined constructor does not initialize 'openPMD::Datatype openPMD::detail::BufferedUniquePtrPut::dtype'

* More careful documentation of streaming API

* Doc: Fix Bib Authors

Make sure the bib authors match the quoted openPMD-standard authors.

* Update .readthedocs.yml

Update to newer Ubuntu, shipping a newer OpenSSL

* Fix deprecated storeChunk APIs in first read/write examples

* CI: macOS-11 Update

The older macOS image is now removed. The latest points already
to macOS-12. macoS-13 runners are marked experimental.

https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources

* Docs: Linking to C++ Projects

Move a section only written in the README to our
developer section on readthedocs.

* Use lazy imports for dask and pandas

* Better error message when loading to a buffer with mismatched type

* Remove schema 2021 from documentation

It will be removed and should no longer be advertised.

* Document ROMIO/HDF5/Chunking issue

open-mpi/ompi#7795

* Use a "minor" RST link

Co-authored-by: Axel Huebl <[email protected]>

* HDF5: HDF5_DO_MPI_FILE_SYNC

Document a new work-around option for MPI-parallel HDF5 for
filesystems that are super limited for paralle I/O features
relevant in HPC.

* Fix typo

Co-authored-by: Franz Pöschel <[email protected]>

* openpmd-pipe: set correct install permissions

* Fix headers in workflow.rst

* Fix documentation for preferred_flush_target in ADIOS2

* HDF5: Fix Char Type Matching

In HDF5, there are only the signed and unsigned char type.
The third `char` type is an alias to one or the other, different
to the C/C++ fundamental types.

This tries to fix the type matching order to be platform independent
between ppc64le/aarch64/x86-64.

* Add failing HDF5 test

* loadChunk(): consider equivalent char types for casting

* Doc strings

* newline & comment

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Doc: Sphinx Copybutton and Design

Add the Sphinx extensions `copybutton` (copy code blocks with
a single click) and `design` (for boxes, tabs, dropdowns, etc.).

* CI: Doxygen 1.9.7 Broken

Markdown support for main file broken in 1.9.7.
Go back to previous patch release. Upstream already fixed.

* Docs: Analysis

Add a data analysis & visualization section.
This is meant to show entry points and workflows to work with
openPMD data in larger frameworks and compatible ecosystems.

* [Draft] DASK, Pandas, ...

* Doc: DASK

* Pandas

* RAPIDS

* Typos

* Don't require unitSI when reading patch record component

* CI: oneAPI 2023.2.0

Update CI to breaking `apt` changes in the latest oneAPI release.

* Update __repr__ method of major objects in openPMD hierarchy

* Deal with trailing slashes in file paths

Happens in bash completion on BP files since they are folders.

* Throw better error messages when selecting a bad backend

* Adapt CoreTest to new error message

* Follow-up to #1470

Changes there left the PatchRecordComponent dirty after parsing

* HDF5IOHandler: Support for float128 data types with 80bit precision on ARM64/PPC64

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Cleanup

Unify little/big endian, use double80 also in other places

Long double size 8 -> 16

Use malloc to avoid alignment issues

Same treatment for complex long double

Add this for readAttribute too

Avoid non-native datatypes in writing

* Make this fix little-endian only

* Add comment

* Suggestions from review

* Use new instead of malloc everywhere

Also, add a more explanative comment

* CI fix: type comparison in Python

No idea why this check is triggered by this PR

* ADIOS2: Ensure that a step is always active at write time

Even when not using steps, dump everything into a single large step. BP5
will fail otherwise.

* CI: macOS 11.0+

We do not have the runners anymore to deploy and test macOS 10.15,
so we bump our tests to 11.0+, too.

Technically, we could build & deploy a bit longer on GH actions
`macos-11` for `10.15` (Catalina), but since it is unmaintained by
Apple/end-of-life already and macOS-11 is the oldest still maintained
OS by Apple, we do also stop support for it.

At the time of writing, old and maintained are:
- macOS 11, Big Sur
- macOS 12, Monterey

latest is: macOS 13, Ventura - and in preview is macOS 14, Sonoma.

This also unifies our arm64/aarch64 (M1/M2) requirements, which are
macOS 11.0+ as well.

* HDF5: Throw ReadError at dataset open

* Try parent types when a dataset datatype is unknown

* Update Sample Download Scripts

* Add test for old HDF5-plugin written openPMD dataset from PIConGPU

* Remove debugging comments

* Warn on BP5+Blosc in ADIOS2 v2.9 up to patch level 1

* Fix unused parameter

* Update Warning String

* Throw error when steps are needed but not supported

* Fix variableBasedSingleIteration test

* Test that the error is correctly thrown

* Introduce Series::parseBase alias for readIterations(), fix workflow

* Have only one instance of SeriesIterator

* Break memory cycle

* Use simple API in test again

* Snapshot attribute in file-based encoding

Snapshot attribute must be written in Iteration::endStep() in file-based
encoding

* Optional debugging output for AbstractIOHandlerImpl::flush()

* Revert "Optional debugging output for AbstractIOHandlerImpl::flush()"

This reverts commit ee8de45.

* Post-rebase fixes

1) Don't write snapshot attributes during initialization of an iteration
2) Catch unsuccessful flush run also in beginStep()

* Ensure that m_lastFlushSuccessful is always called

* Pandas DataFrames: Add Row Column Name

By default, the row index (!= particle index) in a pandas
dataframe has no name. This can be a bit cumbersome for
exports, e.g., to CSV - where this header field would just
be empty.

This PR names the index now "row", because it is not a
(macro) particle id property.

* Python: 3.8+

Python 3.7 went EOL last month. This bumps our supported
versions to 3.8+.

* Optional debugging output for AbstractIOHandlerImpl::flush()

* Add an environment variable for this

* Version 0.15.2 + Changelog

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Python: Fix Series Compile

Fix compile issue introduced during backporting.

* Unmerge 0.16 content from variableBasedSeries test

* Reintroduce backend selection in variableBasedSeries test

Still needed due to ADIOS1 BP env variable

* Add src/Dataset.cpp to ADIOS1 source

* Replace openPMD_Datatypes global with function

* Windows CI: Bump Version to 0.15.2

* OPENPMD_BP_BACKEND=="ADIOS1": Skip BP5 Tests

* Streaming examples: Set WAN as default transport

* Exclude ADIOS1 from variableBasedSeries test

* Changelog: Bump Date

* replace extent in weighting and displacement

store extent value in n_particles

* Examples: Fix Types of Constants & Attributes

Backports from #1316 and #1510

* Changelog: Streaming Example Note

* CMake: Warn and Continue on Empty HDF5_VERSION

Seen on Conda-Forge for arm64 on macOS for HDF5 1.14.1

---------

Co-authored-by: Franz Pöschel <[email protected]>
Co-authored-by: Dave Grote <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Ulrik Guenther <[email protected]>
Co-authored-by: Kara-Mostefa Ilian <[email protected]>
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