Fix HDF5Writer id()-reuse bug corrupting class_index in multi-worker generation - #479
Open
sclkim1 wants to merge 2 commits into
Open
Fix HDF5Writer id()-reuse bug corrupting class_index in multi-worker generation#479sclkim1 wants to merge 2 commits into
sclkim1 wants to merge 2 commits into
Conversation
…data HDF5Writer stamped a stable, counter-based _hdf5_key on each Signal object before writing, but not on the signal's metadata .parent chain (generator/ class-level metadata) - those objects fell back to str(id(obj)) in _hdf5_key(). Parent objects aren't guaranteed to stay alive for the whole write session, so CPython can recycle a freed parent's memory address for a later, unrelated parent; both then produce the same HDF5 key, and populate_hdf5_group_with_metadata()'s "if key in group: return False" guard silently keeps the first parent's data and skips writing the second - so every signal pointing at the second parent reads back the first parent's fields (most visibly, class_index coming back as a list instead of a scalar). Reproduced via WorkerSeedingDataLoader + DatasetCreator with multiple workers (0% single-process, ~1.4% at 3K samples, ~67% on a real 1.06M-sample run). Fixed by also stamping a stable key on the parent chain (_assign_hdf5_keys_to_parent_chain), reusing an existing key via hasattr so a genuinely-shared parent object still dedupes to one HDF5 group. Verified: the same 50K-sample/10-worker repro that showed corruption now shows 0/50000 bad. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…use bug Generates 3000 samples through the real WorkerSeedingDataLoader/DatasetCreator multi-worker path (the scale/worker-count combination that reliably reproduced the bug pre-fix) and asserts no sample's class_index comes back as a list. See the previous commit and issue TorchDSP#478 for the root cause. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
sclkim1
force-pushed
the
fix/hdf5-writer-parent-key-corruption
branch
from
August 25, 2026 11:29
cbeeda0 to
2e7e444
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
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.
Fixes #478.
Problem
HDF5Writerstamps a stable, counter-based_hdf5_keyon eachSignalobject before writing (_assign_hdf5_keys), but not on the signal's metadata.parentchain (the generator/class-levelHierarchicalMetadataObjectthatpopulate_hdf5_group_with_metadata()recurses into). Those parent objects fell back tostr(id(obj))in_hdf5_key().Parent metadata objects aren't guaranteed to stay alive for the whole write session - a new one can be built per batch/worker and then garbage-collected once its signals are written. When CPython recycles a freed parent's memory address for a later, unrelated parent object, both resolve to the same key, and
populate_hdf5_group_with_metadata()'s dedup guard (if key in group: return False) silently keeps the first parent's data and skips writing the second. Every signal pointing at the second (skipped) parent then reads back the first, unrelated parent's metadata fields on load - most visibly,class_indexcoming back as a list like[correct_label, wrong_label]instead of a scalar.This only shows up with
num_workers > 1(real-scale generation viaWorkerSeedingDataLoader), and the corruption rate grows with run size: 0% single-process, ~1.4% at 3,000 samples/4 workers, ~67% on a real 1,060,000-sample production run in our case. It is not specific to any signal type or impairment level.See #478 for the full writeup.
Fix
HDF5Writer._assign_hdf5_keys()now also stamps a stable key on the metadata.parentchain (_assign_hdf5_keys_to_parent_chain), reusing an existing key viahasattrso a genuinely-shared parent object still dedupes to one HDF5 group - only objects without a key yet (i.e. those at risk of an id() collision) get a fresh counter-based one.Testing
test_writer_no_parent_metadata_id_reuse_corruption(tests/utils/test_writer.py, marked@pytest.mark.full): generates 3,000 samples through the real multi-workerWorkerSeedingDataLoader/DatasetCreatorpath and asserts no sample'sclass_indexcomes back as a list. Passes locally after the fix.