Deprecate Iteration.closed attribute#1088
Merged
ax3l merged 2 commits intoopenPMD:devfrom Oct 19, 2021
Merged
Conversation
franzpoeschel
commented
Aug 16, 2021
| // maybe re-read | ||
| auto & i = series.iterations.at( index ); | ||
| if( guardClosed && i.closedByWriter() ) | ||
| if( guardAgainstRereading && i.written() ) |
Contributor
Author
There was a problem hiding this comment.
I think that this logic (already before this PR) breaks deferred parsing in the following situation:
- group-based interation encoding
- no steps enabled in the backend
- streaming-based API used for reading
I have a feeling that Series::readGorVBased will read all iterations upon the second time being called.
Contributor
Author
There was a problem hiding this comment.
No, all is fine. I've added a test for it.
franzpoeschel
changed the title
franzpoeschel
force-pushed
the
remove-iteration-closedbywriter
branch
from
5339797 to
e802dd7
Compare
ax3l
approved these changes
Sep 22, 2021
Member
|
Discussed today: let's split PR into a backportable bugfix and a deprecation PR so we can backport the bugfix to 0.14.3 |
Contributor
Author
Don't write it no more, don't use it no more, mark Iteration::closedByWriter() deprecated.
franzpoeschel
force-pushed
the
remove-iteration-closedbywriter
branch
from
e802dd7 to
5181315
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Iteration::closedByWriterThis attribute was introduced during the work on our streaming API when openPMD iterations and ADIOS steps were not yet coupled as concepts. As a result, readers needed a way to know when an iteration was fully defined, for this, we introduced this attribute.
Nowadays, iterations are fully defined as soon as the reader sees them.
TODO:
iteration.closedByWriter()withiteration.written()is fine in our reading routines. (Should be fine, I want to carefully go through it in the debugger to be sure anyway).EDIT: Yes, it's possible, but it turns out that the streaming read API is implemented inefficiently for groupbased series that don't use steps, because all iterations will be parsed anew in every step. This PR fixes that.