Skip to content

Make shape an optional attribute for constant components #1661

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 19 commits into
base: dev
Choose a base branch
from
Open

Make shape an optional attribute for constant components #1661

wants to merge 19 commits into from

Conversation

franzpoeschel
Copy link
Contributor

@franzpoeschel franzpoeschel commented Aug 7, 2024 •
edited
Loading

Fix #1663

TODO:

  • Actually test this
  • Link to standard update: Const Records: Relax shape for Particles openPMD-standard#289
  • Maybe only do this for standard version 2.0 - actually ok as a fallback to expose in 1.X files, too UPDATE: This comment explains a corner case that makes me hesitate adding this for version 1.0 since it might break datasets in singular cases. UPDATE: Corner case not relevant enough, would imply misuse of reserved value attribute, so not strictly standard-breaking.
  • For particles only or for meshes too?
  • Test that the shape can be queried from every component by the reader, even if shape attribute is missing
  • Document compatibility option

@ax3l ax3l self-requested a review August 27, 2024 20:53
@ax3l ax3l added this to the 0.17.0 milestone Aug 27, 2024
@ax3l ax3l requested a review from guj August 27, 2024 20:53
@ax3l ax3l mentioned this pull request Aug 27, 2024
Open
6 tasks
@franzpoeschel franzpoeschel mentioned this pull request Sep 6, 2024
Open
@franzpoeschel franzpoeschel force-pushed the dont-require-shape-for-constant-components branch from 0c031dd to 11425ce Compare October 11, 2024 15:08
@franzpoeschel franzpoeschel force-pushed the dont-require-shape-for-constant-components branch from 3764e01 to 9f2474b Compare November 15, 2024 14:12
@franzpoeschel franzpoeschel force-pushed the dont-require-shape-for-constant-components branch 2 times, most recently from 96a8dce to 1b835b2 Compare December 17, 2024 10:49
@franzpoeschel franzpoeschel force-pushed the dont-require-shape-for-constant-components branch from 1b835b2 to ded204d Compare February 21, 2025 10:19
@guj
Copy link
Contributor

guj commented Feb 21, 2025

Thanks I will give it a try!

guj
guj approved these changes Mar 5, 2025
View reviewed changes
Copy link
Contributor

@guj guj left a comment

Choose a reason for hiding this comment

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

I just tried with joined array, it worked!

@franzpoeschel franzpoeschel force-pushed the dont-require-shape-for-constant-components branch from ded204d to 5e0fcbd Compare March 26, 2025 10:17
@franzpoeschel franzpoeschel force-pushed the dont-require-shape-for-constant-components branch 2 times, most recently from 6dfb00e to d63f5ad Compare April 4, 2025 08:31
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 4, 2025
View reviewed changes
test/SerialIOTest.cpp
@@ -768,6 +768,11 @@
#endif
}

TEST_CASE("components_without_extent", "[serial]")

Check notice

Code scanning / CodeQL

Unused static function Note test

Static function C_A_T_C_H_T_E_S_T_18 is unreachable (
autoRegistrar19
Loading
must be removed at the same time)
@franzpoeschel franzpoeschel force-pushed the dont-require-shape-for-constant-components branch from 0a30b07 to 329d77e Compare April 7, 2025 16:10
@franzpoeschel franzpoeschel force-pushed the dont-require-shape-for-constant-components branch from 07c7f2e to d37c7a2 Compare April 22, 2025 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guj guj guj approved these changes

@ax3l ax3l Awaiting requested review from ax3l

Assignees
No one assigned
Labels
backend: ADIOS2 discussion frontend: C++17
Projects
None yet
Milestone
0.17.0
Development

Successfully merging this pull request may close these issues.

Constant Records for JoinedArrays: Skip shape for Particles
3 participants
@franzpoeschel @guj @ax3l