Repository navigation
test(rocfft): stable, position-aware hash for bitwise repro buffers - #12752
Conversation
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #12752 +/- ##
===========================================
- Coverage 71.86% 70.66% -1.20%
===========================================
Files 2876 2939 +63
Lines 475315 483978 +8663
Branches 70237 69789 -448
===========================================
+ Hits 341569 341963 +394
- Misses 109960 118257 +8297
+ Partials 23786 23758 -28
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Any reason to use yet another hashing algorithm (splitmix64 finalizer) instead of the already available SHA-256 reference from shared/sha256.h ? |
@eng-flavio-teixeira SHA-256 reads data in order, so hashing each partition with it changes the fingerprint when the thread count changes. Hashing each element with it is ~186× slower, and hashing the whole buffer would mean redesigning the layout handling and the repro-db format. |
Thanks for the explanation. Can you run the bitwise repro tests twice, passing the repro-db option (if you haven't done it already) to make sure the tests pass without issues with your changes? I don't think a regular CI pass exercise this. |
regan-amd
left a comment
regan-amd
left a comment
There was a problem hiding this comment.
Please make sure hipfft/shared and rocfft/shared contents are aligned before merging.
|
@regan-amd Done in 3e957fb, both points as you suggested:
New tests in
All |
|
@eng-flavio-teixeira Two-process
Run 1 printed |
Abuudiii
changed the title
test(rocfft): stable, position-aware hash for bitwise repro buffers (#12752) ## Motivation JIRA ID : AIFFT-517 The bitwise reproducibility tests store a fingerprint (hash) of each FFT result and compare it on later runs. The hash had three problems: - It used `std::hash`, which can give different results on different compilers or runs, so stored fingerprints could stop matching even when the output was identical. - Each value was multiplied by a weight and rounded before hashing, so a change in the last bit could be lost. - Each element's position was computed as `row + column`, so a transposed output produced the same hash. ## Technical Details - `shared/fft_hash.h`: each element is hashed from its exact bits together with its real position (batch, then row-major index), using a fixed hash function. Results are still added together, so the thread count doesn't affect the hash. - `projects/hipfft/shared/fft_hash.h`: kept identical to the rocFFT copy. - `clients/tests/buffer_hash_test.cpp`: new tests check that a transpose, a 1-bit change and a batch swap all change the hash, and that padding doesn't. Fixed expected hashes cover half, single and double precision. - `testing.md`: updated to describe the new hash. SHA-256 (`shared/sha256.h`) isn't used because its result depends on the order of the data, so it would change with the number of threads. Existing stored fingerprints must be regenerated. ## Test Plan On MI300X (gfx942), ROCm 7.2.4: - Ran the new tests against the old hash and the new one. - Ran the bitwise repro test twice in separate processes with `--repro-db`, for 5 FFT cases. ## Test Result - The old hash fails the transpose, 1-bit and padding tests. The new hash passes all hash tests. - Repro-db: the first run stored all 5 fingerprints, and the second run matched all 5. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
TheRock Submodule Bump ActivityNewest first
|
Motivation
JIRA ID : AIFFT-517
The bitwise reproducibility tests store a fingerprint (hash) of each FFT result and compare it on later runs. The hash had three problems:
std::hash, which can give different results on different compilers or runs, so stored fingerprints could stop matching even when the output was identical.row + column, so a transposed output produced the same hash.Technical Details
shared/fft_hash.h: each element is hashed from its exact bits together with its real position (batch, then row-major index), using a fixed hash function. Results are still added together, so the thread count doesn't affect the hash.projects/hipfft/shared/fft_hash.h: kept identical to the rocFFT copy.clients/tests/buffer_hash_test.cpp: new tests check that a transpose, a 1-bit change and a batch swap all change the hash, and that padding doesn't. Fixed expected hashes cover half, single and double precision.testing.md: updated to describe the new hash.SHA-256 (
shared/sha256.h) isn't used because its result depends on the order of the data, so it would change with the number of threads.Existing stored fingerprints must be regenerated.
Test Plan
On MI300X (gfx942), ROCm 7.2.4:
--repro-db, for 5 FFT cases.Test Result
Submission Checklist