fix(rpp): Fix normalize HOST to write 3D output and zero-init ND reductions - #12205
Open
sruthi0107 wants to merge 2 commits into
Open
sruthi0107 wants to merge 2 commits into
sruthi0107 wants to merge 2 commits into
Conversation
The F32 3D path computed mean/stddev but only dispatched a normalize write for NHWC layouts (avx_axis3 / nontoggle / axis3-toggle). For a same-layout non-NHWC tensor (e.g. NCHW) none of those branches fired, so the destination was left at its 0.0 pre-fill and the op returned all zeros while reporting RPP_SUCCESS. Add a same-layout fallback that uses normalize_3D_tensor_nontoggle, which steps outer dims by the descriptor strides and the innermost contiguously and is therefore layout-agnostic for dense tensors. Fixes the F32 3D "writes zeros" cases (cms2/cms3 3D go green; cms0/cms1 3D now write real values, their residual failures belong to the separate per-sample-params and supplied-stddev issues). The 4D / generic-ND compute-path defect in the same ticket is a distinct root cause and is not addressed here.
compute_ND_mean and compute_ND_stddev accumulate into meanTensor / stdDevTensor with +=, but the generic-ND path never zeroed those buffers first. They still hold the caller-supplied mean/stddev values (the API takes one buffer that is both input and scratch), so the reductions were computed on top of that data: the variance in particular was inflated by the pre-existing stddev, e.g. 1.5 / sqrt((1.0 + sumsq)/N) instead of 1.5 / sqrt(sumsq/N). The 2D and 3D helpers already zero their accumulators (meanPtr[i] = 0 / stdDevPtr[i] = 0) and HIP hipMemsetAsync's the tensors; only the ND path was missing it. Zero meanTensor and stdDevTensor over [0, size) before each compute_ND_* call, in both the f32/f32 and generic host entry points. The contamination shrinks relative to the true sum as more axes are reduced, which is why the error was largest for single-axis masks and vanished at full reduction. Fixes the generic-ND compute cases in the normalize suite (F32 3D/4D and F16 2D/3D/4D partial-mask cms2/cms3), 25 cases in total, with no regressions and HIP unchanged.
✅ 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.
Motivation
Normalize processes a batch of samples, computing or reading a mean and stddev for each, then writing
(src - mean) * scale/stddev + shift. Two independent HOST defects made this fail above 2D:3D output was all zeros. The F32 3D path computed the statistics but only dispatched a normalize write for NHWC layouts. For a same-layout non-NHWC tensor (e.g. NCHW) no branch matched, so the destination was left at its 0.0 pre-fill and the op returned all zeros while reporting RPP_SUCCESS.
4D and F16 values were wrong (not zero). The generic-ND path computes mean/stddev with
compute_ND_mean/compute_ND_stddev, which accumulate with+=. But the destination buffers were never zeroed first, and the API uses one buffer as both input and scratch — so the reductions were computed on top of the caller-supplied values, inflating the variance (scale / sqrt((suppliedStddev + sumsq)/N)).HIP is correct for both and is used as the reference.
Issue
JIRA ID : AICV-224
Technical Details
HOST only; both fixes in
normalize.cpp.3D write: add a same-layout fallback in the 3D dispatch that uses
normalize_3D_tensor_nontoggle, which steps outer dims by the descriptor strides and the innermost contiguously and is therefore layout-agnostic for dense tensors.ND zero-init: zero
meanTensorandstdDevTensorover[0, size)before eachcompute_ND_*call, in both the f32/f32 and generic host entry points, so the accumulating reductions start from zero instead of the caller's supplied values.Test Plan
Ran the normalize correctness filters from the GoogleTest suite:
rpp_tests --gtest_filter='*NormalizeTest*HOST_*cms3*'Test Result