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

No standard-conforming way to define a Series that only has meshes (resp. particles) in some iterations #253

Open
franzpoeschel opened this issue Mar 16, 2021 · 2 comments
Assignees
Labels
major change non-backwards compatible change
Milestone

Comments

@franzpoeschel
Copy link

Issue first noticed while working on this PR for the openPMD API.
Quoting the technical files on meshesPath and particlesPath:

  • note: if this attribute is missing, the file is interpreted as if it contains no mesh records! If the attribute is set, the group behind it must exist!
    […]
  • note: if this attribute is missing, the file is interpreted as if it contains no particle records! If the attribute is set, the group behind it must exist!

Since both these attributes live at the root level of the hierarchy and cannot be defined specifically per iteration, this makes it impossible to have a Series where only some, but not all iterations contain meshes/particles. If I'm not mistaken, this holds true for file-based as well as group-based iteration layout.
So, I suggest replacing

the group behind it must exist

with

the group behind it may or may not exist

Reading software would then need to check for group existence.

@ax3l ax3l self-assigned this Mar 17, 2021
@ax3l ax3l added the major change non-backwards compatible change label Mar 17, 2021
@ax3l ax3l added this to the openPMD 2.X milestone Mar 17, 2021
@ax3l
Copy link
Member

ax3l commented Mar 17, 2021

Thank you for raising this. We introduced the clarification in openPMD 1.1 to ease readers, but the choice should indeed be considered a defect.

It's easier to check for existence at read time than the problem it introduces at write time. I am still thinking if we should also mak the attribute optional, which is clearer for purely mesh- or particle-containing series. This then means we need at read time two checks: attribute exists (y/n) - no means no meshes/particles in the series - and then value points to something existing (y/n).

@franzpoeschel
Copy link
Author

This then means we need at read time two checks: attribute exists (y/n) - no means no meshes/particles in the series - and then value points to something existing (y/n).

So, a one-sided implication?
Current rule: mesh group exists <-> meshesPath is set
Your suggestion: mesh group exists -> meshesPath is set

If yes, sounds good with me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major change non-backwards compatible change
Projects
Status: Proposed
Development

No branches or pull requests

2 participants