[hipDNN] Test cpu graph executor for batch norm inference variance#3959
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances test coverage and validation for the batch normalization inference with variance feature. It adds comprehensive CPU graph executor tests and fixes a validation gap in the frontend node.
Changes:
- Added three new tests (
ExecutePlan,PlanConstruction,IsApplicable) forBatchnormFwdInferenceWithVariancePlanin the test SDK - Fixed missing epsilon tensor validation in
BatchnormInferenceNodeVarianceExt::pre_validate_node - Extended test utilities (
BatchnormGraphUtilsandBatchnormTensorBundles) to support variance-based batch normalization tests
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| TestBatchnormFwdInferenceWithVariancePlan.cpp | Adds comprehensive tests for plan execution, construction, and applicability checks |
| BatchnormTensorBundles.hpp | Introduces BatchnormFwdWithVarianceTensorBundle struct for test tensor management |
| BatchnormGraphUtils.hpp | Adds buildBatchnormFwdInferenceWithVarianceGraph helper function for test graph creation |
| BatchnormFwdInferenceWithVariancePlan.hpp | Adds epsilon tensor existence validation to plan builder |
| TestBatchnormInferenceNodeVarianceExt.cpp | Adds test case for missing epsilon tensor validation |
| BatchnormInferenceNodeVarianceExt.hpp | Adds epsilon tensor validation check to prevent incomplete node configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
BrianHarrisonAMD
left a comment
There was a problem hiding this comment.
Looks good assuming tests pass.
I think it would be worth double checking the value init, and epsilon pass-by-value checks in frontend, but I don't think it's blocking.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #3959 +/- ##
============================================
+ Coverage 49.42% 81.13% +31.71%
============================================
Files 135 156 +21
Lines 33698 15865 -17833
Branches 4436 1993 -2443
============================================
- Hits 16654 12871 -3783
+ Misses 15865 2216 -13649
+ Partials 1179 778 -401
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…rm-variance-tests
…sts' of github.com:ROCm/rocm-libraries into users/bghimire/cpu-graph-executor-batchnorm-variance-tests
|
Issuing override merge. Lots of slowdowns on windows gfx1151. This is being worked on and this PR doesnt need to be adding to the queue times. |
Motivation
Added missing tests for
BatchnormFwdInferenceWithVariancePlan.Fixed incomplete validation in
BatchnormInferenceNodeVarianceExtwhich was missing the check for theepsilontensor, ensuring standard validation catches missing inputs.Test Plan
Verify CPU Graph Executor tests (Execution, Plan Construction, Applicability)
./bin/hipdnn_test_sdk_tests --gtest_filter="*BatchnormFwd*VariancePlan*"Verify Frontend Validation tests (Missing Epsilon check)
./bin/hipdnn_frontend_tests --gtest_filter="*BatchnormInferenceNodeVarianceExt*"Test Result
Submission Checklist