Skip to content

Conversation

franzpoeschel
Copy link
Contributor

@franzpoeschel franzpoeschel commented Aug 6, 2024

Follow-up to #1619
This lets users decide to flush only the containing Iteration or everything with Attributable::seriesFlush().

TODO:

  • Maybe use false as a default even. Would be a breaking change, but I'd argue it's the more intuitive behavior and most usage is probably along that line. They're two different functions now.

@franzpoeschel franzpoeschel added api: new additions to the API workaround labels Aug 6, 2024
@franzpoeschel franzpoeschel added this to the 0.16.0 milestone Aug 6, 2024
@franzpoeschel franzpoeschel requested review from ax3l and guj August 6, 2024 16:06
@franzpoeschel franzpoeschel force-pushed the more-specific-iteration-flush branch from dbbaa36 to ed91f9e Compare August 6, 2024 16:20
@guj
Copy link
Contributor

guj commented Aug 7, 2024

Thanks Franz.

  1. It seems to me that for an I/O library that requires collective efforts, e.g. BP5, it is better to use iteration->seriesFlush() instead of series->flush(). So, should the default behavior of series->flush() be flushing everything, and set a flag like "dirtyonly= true" if optimization is desired.

  2. naming is a bit confusing to me. by calling iteration->seriesFlush(flush_entire_series=true), it will flush all other iterations in the underlying series owning this iteration? might as well have something like series->flush(dirtyonly=false)? i.e. I think letting iteration/series flush itself only would be cleaner.

@ax3l
Copy link
Member

ax3l commented Aug 27, 2024

Good points. I agree, we could make it maybe more transparent by introducing iteration->flush() and keeping iteration->seriesFlush() to point to the series flush?

@franzpoeschel
Copy link
Contributor Author

In that case, I'd suggest to add Attributable::iterationFlush() instead of Iteration::flush():

  • more intuitively tells the meaning of Attributable::seriesFlush() in opposition to Attributable::iterationFlush()
  • users don't need to carry around the Iteration for flushing

@franzpoeschel franzpoeschel force-pushed the more-specific-iteration-flush branch from 280030b to 637c549 Compare September 6, 2024 13:52
@ax3l ax3l self-assigned this Sep 17, 2024
@ax3l ax3l merged commit ff0c669 into openPMD:dev Sep 17, 2024
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: new additions to the API workaround

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants