Fix Iteration::open(), needed for correct use of Span API#1794
Fix Iteration::open(), needed for correct use of Span API#1794franzpoeschel merged 4 commits intoopenPMD:devfrom
Conversation
docs/source/details/mpi.rst
Outdated
| .. [4] We usually open iterations delayed on first access. This first access is usually the ``flush()`` call after a ``storeChunk``/``loadChunk`` operation. If the first access is non-collective, an explicit, collective ``Iteration::open()`` can be used to have the files already open. | ||
| Alternatively, iterations might be accessed for the first time by immediate operations such as ``::availableChunks()``. | ||
|
|
||
| .. [5] The Span-based ``storeChunk`` API calls return a backend-allocated pointer, requiring flush operations. These API calls hence inherit the collective properties of flushing. Use store calls with zero-size extent if a rank does not contribute anything. |
There was a problem hiding this comment.
cc @guj
Oh, but that has some severe implications for particle flushes and codes with dynamic load balancing, where one does have a non-equal number of calls to storeChunk.
- Fields: generally, non-equal number of calls per MPI rank use span to allocate mesh data by default BLAST-WarpX/warpx#6123
- Particles: some ranks write zero particles, some call multiple times Optimizations for BP5 engine of ADIOS2: Use span-based API and avoid flushing before closing an Iteration BLAST-WarpX/warpx#3200
This makes usage very complicated for codes like WarpX.
@franzpoeschel Maybe we can relax the constraint t keep storeChunk indepdnent - instead put more logic in resetDataset or so?
There was a problem hiding this comment.
Alternative:
- Introduce
RecordComponent::open()to do all necessary collective preparations - Check that
SkeletonOnlyflush does nothing collective
There was a problem hiding this comment.
Discussed:
- collective component only comes in for the first
storeChunk(that means only particles would have a problem in most cases) - there is some confusion in the table there between the MPI concepts: collective/independent (described) and synchronizing Doc: MPI Sync? #1796 (not described)
There was a problem hiding this comment.
Franz comment: Collective might only be the ADIOS engine open -- we probably do not need at all to delay this and could do this collectively and blocking in Series open...?
To double check if Iteration::open() really opens the engine (x-ref)
There was a problem hiding this comment.
is collective a performance boost requirement:
There was a problem hiding this comment.
Okay, we can keep the current API:
Iteration::open()must be used in parallel setups to ensure collective file access- This was not working properly in random-access mode, now fixed
- The Span API can then be used non-collectively
There was a problem hiding this comment.
Oh and:
- Due to the bug mentioned above, current releases require a workaround: Each rank must contribute at least one
storeChunk()operation that may be empty.
There was a problem hiding this comment.
Oh and:
- Due to the bug mentioned above, current releases require a workaround: Each rank must contribute at least one
storeChunk()operation that may be empty.
so span should require the next release version?
There was a problem hiding this comment.
That's probably for the best, yeah. For older versions, you can alternatively include the workaround described above.
franzpoeschel
force-pushed
the
fix-collective-span-api
branch
from
dfc8cea to
58c08ea
Compare
franzpoeschel
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| IOHandler()->flush(internal::defaultFlushParams); | ||
| break; | ||
| } | ||
| // IOHandler()->flush(internal::defaultFlushParams); |
Check notice
Code scanning / CodeQL
Commented-out code Note
|
Thank you so much @franzpoeschel and @guj , this is great! Let's do a new release soon? :) |
3 participants
Seen on #1791, fixes:
Iteration::open()in random-access mode (did not open files, leading to later hangups due to non-collective file opening)