Skip to content

Fix HDF5 dataset re-creation on repeated whole-IDS writes to an open pulse - #100

Open
olivhoenen wants to merge 3 commits into
split85/5-presets-docs-fixturesfrom
split85/6-fix-hdf5-repeated-writes
Open

olivhoenen wants to merge 3 commits into
split85/5-presets-docs-fixturesfrom
split85/6-fix-hdf5-repeated-writes

Conversation

@olivhoenen

Copy link
Copy Markdown
Collaborator

Part of the split of #85 (Tests and Benchmarks for regression) into smaller, single-purpose PRs that are easier to review.

Bug fix. Writing a whole IDS twice to the same open HDF5 pulse failed: write_ND_Data only consulted the per-action cache of open datasets, so on the second action it tried to create a dataset that already existed on disk. A new helper, HDF5Writer::openOrCreateNonSliceDataSet, opens and extends an existing dataset instead of re-creating it (data and _SHAPE datasets). Includes the regression tests in test_structured_data.cpp (ASCII skipped, as it cannot read arrays of structures back).

Fork history: found while writing the whole-IDS write benchmark (yohannmarguier#59, yohannmarguier#60).

Stack position: 6/9; depends on # (only for the test harness). The benchmark PR builds on this one.

Authorship: these commits are @yohannmarguier's, cherry-picked unchanged from his fork with git cherry-pick -x, so each one keeps its original author, date and message plus a (cherry picked from commit …) link back to the fork. Only conflicts with current develop and with the new commit order were resolved.

⚠️ Please merge with "Create a merge commit" or "Rebase and merge", not "Squash and merge": squashing would attribute Yohann's commits to the person who merges.

🤖 Generated with Claude Code

…pulse (issue #60)

HDF5Writer::write_ND_Data and ::createOrUpdateShapesDataSet decided
create-vs-reuse for a dataset purely from the per-action opened_data_sets
cache, which is cleared at the end of every al_end_action(WRITE_OP) even
though the underlying HDF5 file (and its datasets) persists for as long
as the pulse stays open. A second global write on the same still-open
pulse therefore always took the "not yet created" branch and called
H5Dcreate2 on a dataset that already existed, throwing "Unable to create
HDF5 dataset". Both call sites now check H5Lexists first and open() the
existing dataset instead of re-creating it, mirroring the SLICE_OP branch
a few lines below (and createOrUpdateAOSShapesDataSet, a third function
that already had this guard).

Adds EquilibriumSeedMatrix.RepeatedWholeIdsWriteOnSameOpenPulseSucceeds
to pin the fix: two al_begin_global_action(WRITE_OP) calls on the same
open pulse, reproducing the exact failure shape independent of which
benchmark or caller triggers it. Verified by reverting the fix alone and
confirming the new test fails with the original error message.

(cherry picked from commit 353a66d)
(cherry picked from commit 8ab1815)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants