fix(hipdnn): move SdpaFwd test bundles into the post-rename directory - #11481
Open
zhihuidu-amd wants to merge 1 commit into
Open
fix(hipdnn): move SdpaFwd test bundles into the post-rename directory#11481zhihuidu-amd wants to merge 1 commit into
zhihuidu-amd wants to merge 1 commit into
Conversation
PR ROCm#10162 renamed dnn-providers/integration-tests/integration_test_bundles to integration-test-bundles. PR ROCm#9589 was in flight across that rename and recreated the old path, so its three Flash2 golden-reference bundles (hd128_causal_gqa4, hd128_causal_mha, hd64_causal) landed in a directory nothing reads. They were the only content left there. Reported by @ScottTodd on ROCm#9589: the new directory carries a `*.bin` catch-all in .gitignore so DVC-pulled tensors can never be staged by accident, while the old path has no such rule -- so after `dvc pull` the twelve .bin files show up as untracked for everyone. There is a second consequence that was not reported, and it is the more significant one: **the bundles were never loaded by any test.** dnn-providers/integration-tests/CMakeLists.txt:133 copies only integration-test-bundles/ into the build tree and installs only that at :149, and both BundleRegistration.hpp:214 and main.cpp:134 resolve <exe>/../lib/integration-test-bundles. Nothing in the tree references the underscore path. So these three bundles have been inert since the rename. Pure `git mv` of nine files: 9 renames, 0 insertions, 0 deletions. No code change is needed -- the harness discovers bundles by scanning the directory, not from a manifest, and none of the three names already existed under the new path so there is no collision. DVC is unaffected. Each .tensors.dvc entry addresses its blob by md5 content hash with `path` holding only the bare filename relative to the pointer, so moving the pointer does not change what `dvc pull` fetches. Verified the pointer contents are byte-identical after the move, and that `git check-ignore` now matches the .bin files against integration-test-bundles/.gitignore:4.
✅ 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. |
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
1 participant
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.
PR #10162 renamed dnn-providers/integration-tests/integration_test_bundles to integration-test-bundles. PR #9589 was in flight across that rename and recreated the old path, so its three Flash2 golden-reference bundles (hd128_causal_gqa4, hd128_causal_mha, hd64_causal) landed in a directory nothing reads. They were the only content left there.
Reported by @ScottTodd on #9589: the new directory carries a
*.bincatch-all in .gitignore so DVC-pulled tensors can never be staged by accident, while the old path has no such rule -- so afterdvc pullthe twelve .bin files show up as untracked for everyone.There is a second consequence that was not reported, and it is the more significant one: the bundles were never loaded by any test. dnn-providers/integration-tests/CMakeLists.txt:133 copies only integration-test-bundles/ into the build tree and installs only that at :149, and both BundleRegistration.hpp:214 and main.cpp:134 resolve /../lib/integration-test-bundles. Nothing in the tree references the underscore path. So these three bundles have been inert since the rename.
Pure
git mvof nine files: 9 renames, 0 insertions, 0 deletions. No code change is needed -- the harness discovers bundles by scanning the directory, not from a manifest, and none of the three names already existed under the new path so there is no collision.DVC is unaffected. Each .tensors.dvc entry addresses its blob by md5 content hash with
pathholding only the bare filename relative to the pointer, so moving the pointer does not change whatdvc pullfetches. Verified the pointer contents are byte-identical after the move, and thatgit check-ignorenow matches the .bin files against integration-test-bundles/.gitignore:4.Motivation
Technical Details
Test Plan
Test Result
Submission Checklist