Skip to content

File-based iteration encoding: Re-parse padding when necessary#1081

Merged
ax3l merged 5 commits intoopenPMD:devfrom
franzpoeschel:fix-late-filebased-setting
Aug 13, 2021
Merged

File-based iteration encoding: Re-parse padding when necessary#1081
ax3l merged 5 commits intoopenPMD:devfrom
franzpoeschel:fix-late-filebased-setting

Conversation

@franzpoeschel
Copy link
Contributor

@franzpoeschel franzpoeschel commented Aug 6, 2021

Close #1048

Until now, the methods SeriesInterface::setName and SeriesInterface::setIterationEncoding do not modify the parsed filename prefix, padding and postfix.
Reparse them when necessary and detect wrong specifications.

Meta: We might want to introduce a label middle-end? This is not a user-facing change (except for fixing a bug), so I don't want to add the label Frontend-C++14. It's a change in the implementation of our frontend, i.e. the middle-end.

@franzpoeschel franzpoeschel force-pushed the fix-late-filebased-setting branch from d76a7ba to 3f8de28 Compare August 6, 2021 11:44
@franzpoeschel
Copy link
Contributor Author

franzpoeschel commented Aug 6, 2021

I don't understand this line in the old implementation of setName(). It checks if the old name contains %T right before overwriting it? Why should that make a difference? Is it supposed to be possible to set a name without expansion pattern after initializing a Series with an expansion pattern?

I implemented a prototype of this in commit 9f2c38d, should we go that way?

Series o = Series("./new_openpmd_output_%T.json", Access::CREATE);

o.setOpenPMDextension(42);
o.setIterationEncoding(IterationEncoding::fileBased);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fun fact: This is exactly the kind of wrong usage that #1048 reports. Since this PR adds sanity-checks to catch such behavior, I'm changing this Series to be file-based now.

@franzpoeschel franzpoeschel force-pushed the fix-late-filebased-setting branch from 587183d to 78cd914 Compare August 9, 2021 09:18
@ax3l ax3l self-requested a review August 11, 2021 05:51
@ax3l ax3l self-assigned this Aug 11, 2021
@ax3l
Copy link
Member

ax3l commented Aug 11, 2021

Merged #1080, ready for rebase :)

@franzpoeschel franzpoeschel force-pushed the fix-late-filebased-setting branch from 78cd914 to e159ea0 Compare August 12, 2021 09:21
1) Revert unnecessary changes (include ordering)
2) Update comments
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.

Looks great, thanks a lot!

@ax3l ax3l merged commit 0a57fff into openPMD:dev Aug 13, 2021
@franzpoeschel franzpoeschel deleted the fix-late-filebased-setting branch August 13, 2021 08:18
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.

infinite loop when filename does not contain '%T' but IterationEncoding is fileBased

2 participants