Skip to content

Parallel HDF5: Try fixing strict collective requirements of HDF5 >= 2.0#1862

Draft
franzpoeschel wants to merge 85 commits intoopenPMD:devfrom
franzpoeschel:fix-parallel-hdf5
Draft

Parallel HDF5: Try fixing strict collective requirements of HDF5 >= 2.0#1862
franzpoeschel wants to merge 85 commits intoopenPMD:devfrom
franzpoeschel:fix-parallel-hdf5

Conversation

@franzpoeschel
Copy link
Contributor

@franzpoeschel franzpoeschel commented Mar 11, 2026 •
edited
Loading

It seems that HDF5 has become quite a bit pickier about metadata definitions in parallel setups with versions 2.0 and 2.1, leading to hangups.
Earlier, it was enough to define them consistently across ranks, now we apparently have to keep the exact same order of operations.

This is bad for the Span API which runs internal flushes for structure setup.

  • No longer flush inside the Span API, instead flush already at resetDataset()
  • Try if we can only enqueue operations and run them later, should be fine for the ordering
  • Ensure that only a select type of operation runs in structural setup flushes
  • Remove flushParticlesPath and flushMeshesPath functions, these unnecessarily leaked attribute flushes into the structure setup
  • Fix tests...
  • Guard setDirty calls
  • There are some commits deactivating a handful of tests, deal with them

@franzpoeschel franzpoeschel added this to the 0.17.1 milestone Mar 11, 2026
This was referenced Mar 11, 2026
Merged
Merged
@franzpoeschel franzpoeschel force-pushed the fix-parallel-hdf5 branch 2 times, most recently from 11ba7aa to b6ea4b4 Compare March 12, 2026 15:14
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 12, 2026
View reviewed changes
src/IO/AbstractIOHandlerImpl.cpp
}
} // namespace

std::future<void> AbstractIOHandlerImpl::flush(FlushLevel l)

Check warning

Code scanning / CodeQL

Poorly documented large function Warning

Poorly documented function: fewer than 2% comments for a function of 428 lines.
src/IO/AbstractIOHandlerImpl.cpp
while (!(*m_handler).m_work.empty())
{
IOTask &i = (*m_handler).m_work.front();
// verifyFlushType(i.operation, l);

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
This was referenced Mar 13, 2026
Open
Merged
@franzpoeschel franzpoeschel force-pushed the fix-parallel-hdf5 branch 2 times, most recently from a5c1985 to bcb55ef Compare March 16, 2026 10:12
@franzpoeschel franzpoeschel modified the milestones: 0.17.1, 0.18.0 Mar 16, 2026
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 16, 2026
View reviewed changes
include/openPMD/backend/scientific_defaults/ProcessParsedAttribute.tpp
{
template <typename T, typename RecordType>
PostProcessConvertedAttributeImpl<T, RecordType>::
PostProcessConvertedAttributeImpl(RecordType record_in, handler_t reader_in)

Check notice

Code scanning / CodeQL

Large object passed by value Note

This parameter of type
Mesh
Loading
is 112 bytes - consider passing a const pointer/reference instead.
test/CoreTest.cpp
Comment on lines +698 to +701
REQUIRE(r.numAttributes() == 0);
// TODO: unitSI
// REQUIRE(r["x"].numAttributes() == 0);
// REQUIRE(r["y"].numAttributes() == 0);

Check notice

Code scanning / CodeQL

Commented-out code Note test

This comment appears to contain commented-out code.
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 16, 2026
View reviewed changes
test/ParallelIOTest.cpp
MPI_COMM_WORLD);
i.parseBase();
REQUIRE(i.rankTable(/* collective = */ true) == compare);
// REQUIRE(i.rankTable(/* collective = */ true) == compare);

Check notice

Code scanning / CodeQL

Commented-out code Note test

This comment appears to contain commented-out code.
test/ParallelIOTest.cpp
MPI_COMM_WORLD);
i.parseBase();
REQUIRE(i.rankTable(/* collective = */ false) == compare);
// REQUIRE(i.rankTable(/* collective = */ false) == compare);

Check notice

Code scanning / CodeQL

Commented-out code Note test

This comment appears to contain commented-out code.
test/ParallelIOTest.cpp
Access::READ_RANDOM_ACCESS,
MPI_COMM_WORLD);
REQUIRE(i.rankTable(/* collective = */ true) == compare);
// REQUIRE(i.rankTable(/* collective = */ true) == compare);

Check notice

Code scanning / CodeQL

Commented-out code Note test

This comment appears to contain commented-out code.
test/ParallelIOTest.cpp
Access::READ_RANDOM_ACCESS,
MPI_COMM_WORLD);
REQUIRE(i.rankTable(/* collective = */ false) == compare);
// REQUIRE(i.rankTable(/* collective = */ false) == compare);

Check notice

Code scanning / CodeQL

Commented-out code Note test

This comment appears to contain commented-out code.
test/ParallelIOTest.cpp
*/
auto rankMetaIn = series.rankTable(/* collective = */ true);
OPENPMD_REQUIRE_GUARD_WINDOWS(rankMetaIn == writingRanksHostnames);
// OPENPMD_REQUIRE_GUARD_WINDOWS(rankMetaIn == writingRanksHostnames);

Check notice

Code scanning / CodeQL

Commented-out code Note test

This comment appears to contain commented-out code.
test/ParallelIOTest.cpp
TEST_CASE("adios2_chunk_distribution", "[parallel][adios2]")
{
adios2_chunk_distribution::run_test();
// adios2_chunk_distribution::run_test();

Check notice

Code scanning / CodeQL

Commented-out code Note test

This comment appears to contain commented-out code.
test/SerialIOTest.cpp
if (test_rank_table)
{
REQUIRE(read.rankTable(/* collective = */ false) == compare);
// REQUIRE(read.rankTable(/* collective = */ false) == compare);

Check notice

Code scanning / CodeQL

Commented-out code Note test

This comment appears to contain commented-out code.
test/SerialIOTest.cpp
<< host << "'\n";
}
REQUIRE(rank_table.size() == 1);
// REQUIRE(rank_table.size() == 1);

Check notice

Code scanning / CodeQL

Commented-out code Note test

This comment appears to contain commented-out code.
@franzpoeschel
Fix bug with metadata set too late
5f156ad
@franzpoeschel
Flush structural info of resetDataset() to backend immediately
481f76c 
This makes it easier to keep MPI processes in sync
@franzpoeschel
Erase flushMeshes/ParticlesPath
675bbb7 
These were unnecessary, but they snuck WRITE_ATT tasks into the skeleton flush.
@franzpoeschel
Move flushing from storeChunk to resetDataset
a863f53
@franzpoeschel
Hmm, move CREATE_DATASET task back to storeChunk
1805b5e 
not so great, but lets keep that for now
@franzpoeschel
Fix attribute flushing logic
90e5b44
@franzpoeschel
flush mode helpers
2f008a3
@franzpoeschel
Fix dirty handling
19ca8c7
@franzpoeschel
Add TODO comment
f046e74
@franzpoeschel
WIP Runtime verification of flush level
545452b
@franzpoeschel
dont flush to IO handler yet in resetDataset
0b3ac4b
@franzpoeschel
Revert "dont flush to IO handler yet in resetDataset"
4d15ce4 
This reverts commit 6a5c9f5.
@franzpoeschel
Continue fixing and breaking things..
88a1572
@franzpoeschel
Fix API call after rebase
567d00f
@franzpoeschel
Fix dirty handling filebased
0c176a1
@franzpoeschel
TMP REVERT ME: deactivate span table tests
f118654
@franzpoeschel
TMP REVERT ME take out hanging parallel test
48b4c2a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

backend: HDF5 bug

Projects

None yet

Milestone

0.18.0

Development

Successfully merging this pull request may close these issues.

1 participant

@franzpoeschel