Skip to content

feat(hipdnn): add tensor attribute equality helpers#8589

Open
MarijaGijic wants to merge 10 commits into
ROCm:developfrom
MarijaGijic:feature/tensor-equality-6385
Open

feat(hipdnn): add tensor attribute equality helpers#8589
MarijaGijic wants to merge 10 commits into
ROCm:developfrom
MarijaGijic:feature/tensor-equality-6385

Conversation

@MarijaGijic

Copy link
Copy Markdown

fixes #6385

Motivation

This PR implements structural and strict equality helper methods for TensorAttributes. This lays the crucial foundation for fixing graph-level structural comparisons during graph transformation and lifting phases (as outlined in issue #6385). It allows developers to check if two tensors share the same shape and memory footprint without strictly enforcing internal metadata names.

Technical Details

  • TensorAttributes::LogicallyEquals(): Added a comparison method that validates structural alignment—specifically checking dimensions (dim), stride, and data_type while intentionally omitting names and UIDs.
  • operator== and operator!=: Implemented standard strict equality operators that verify both the underlying logical properties and exact identifier fields (including names and virtual flags).
  • Unit Testing: Added a comprehensive TensorLogicalAndStrictEquality test suite within TestTensorAttributes.cpp to explicitly cover all edge cases (dimension matches, scalar matching, name variations, and virtual status changes).

Test Plan

The changes were compiled and validated locally within therock Docker container environment using the native build tools.

# Clean cache and build targets
cd build && ninja clean && ninja

# Run the specific frontend test suite
./bin/hipdnn_frontend_tests --gtest_filter=TestTensorAttributes.*

@MarijaGijic MarijaGijic requested a review from a team as a code owner June 18, 2026 12:48
@BrianHarrisonAMD

Copy link
Copy Markdown
Contributor

Thanks for the contribution!

Approving workflow run.

@assistant-librarian assistant-librarian Bot added the external contribution Code contribution from users community.. label Jun 18, 2026
Comment thread projects/hipdnn/frontend/tests/TestTensorAttributes.cpp Outdated
Comment thread projects/hipdnn/frontend/include/hipdnn_frontend/attributes/TensorAttributes.hpp Outdated
Comment thread projects/hipdnn/frontend/tests/TestTensorAttributes.cpp Outdated
Comment thread projects/hipdnn/frontend/include/hipdnn_frontend/attributes/TensorAttributes.hpp Outdated
Comment thread projects/hipdnn/frontend/include/hipdnn_frontend/attributes/TensorAttributes.hpp Outdated
@MarijaGijic MarijaGijic force-pushed the feature/tensor-equality-6385 branch from 67322b1 to 39a9ec6 Compare June 19, 2026 12:16

@BrianHarrisonAMD BrianHarrisonAMD left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good thanks for the updates!
Ill trigger CI and merge once it passes shortly, but for now we are delaying merges.

@MarijaGijic MarijaGijic force-pushed the feature/tensor-equality-6385 branch from 046d0e7 to 3168cfa Compare June 22, 2026 08:10
@MarijaGijic MarijaGijic force-pushed the feature/tensor-equality-6385 branch 3 times, most recently from d461273 to d65aef1 Compare June 23, 2026 12:26
@MarijaGijic MarijaGijic force-pushed the feature/tensor-equality-6385 branch from d65aef1 to 08a1cee Compare June 23, 2026 12:32
@BrianHarrisonAMD

Copy link
Copy Markdown
Contributor

Okay updating from latest, and will get this merged once it passes.

@therock-pr-bot

therock-pr-bot Bot commented Jun 29, 2026

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
🌿 Branch Name ❌ Fail Branch name does not match allowed patterns.
Branch: feature/tensor-equality-6385
Allowed patterns:
- ^users\/[A-Za-z0-9][A-Za-z0-9\-]*\/.+
- ^shared\/.+
- ^[A-Za-z0-9][A-Za-z0-9\-_]*$
- ^dependabot\/.+
- ^revert-[0-9]+-.+
📝 PR Title/Description ✅ Pass
Forbidden Files ✅ Pass
🧪 Unit Test ❌ Fail Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/hipdnn/frontend/include/hipdnn_frontend/attributes/TensorAttributes.hpp, projects/hipdnn/frontend/tests/TestTensorAttributes.cpp; no test file found
🔎 pre-commit ⏳ Pending ⏳ Still running…
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled

⚠️ 2 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ Branch Name
  • ❌ Unit Test

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

@therock-pr-bot

therock-pr-bot Bot commented Jun 29, 2026

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ Branch Name
  • ❌ Unit Test

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

@BrianHarrisonAMD BrianHarrisonAMD changed the title [hipDNN] Implement LogicallyEquals and operator== for TensorAttribute… feat(hipdnn): add tensor attribute equality helpers Jun 29, 2026
@BrianHarrisonAMD

Copy link
Copy Markdown
Contributor

@MarijaGijic the changes look good generally, but pre-commit & tidy needs to be passing on this before we can merge.
Can you update this when you get a chance so these checks are passing?

Heads up, there is also #8728 which would require changes to these equality checks depending on which lands first.

@MarijaGijic

Copy link
Copy Markdown
Author

Hi @BrianHarrisonAMD, just a quick update: I’ve resolved all the clang-tidy issues, so I hope the code itself is ready for merge.

The not ready to review label is only remaining because of the PR bot bug ignoring legacy Test*.cpp files. I've opened an issue detailing the bug and a quick configuration fix here: #9010. Could you take a look when you get a chance so we can get this unblocked? Thanks!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[hipDNN] Implement Equality check for Graph, Node, and Tensor to simplify round-trip tests

3 participants