Test suite - Add QA pass/fail tests for F32 bit depth #578
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds float32 QA pass/fail support by inserting range checks and updating testing logic, plus sample reference outputs.
- Removed default
elseassignments in HOST/HIP test runners to ensure QA path is used - Introduced
rpp_pixel_check_0to1calls after float-based color temperature adjustments - Adjusted alignment and pointer arithmetic in glitch and flip kernels for F32
Reviewed Changes
Copilot reviewed 6 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| utilities/test_suite/HOST/runImageTests.py | Removed fallback else block assigning inFilePath1/2, affecting other test cases |
| utilities/test_suite/HIP/runImageTests.py | Converted if to elif for lens_correction and removed redundant else blocks for srcPath defaults |
| src/modules/tensor/hip/kernel/color_temperature.cpp | Added rpp_hip_pixel_check_0to1 call after F32 color temperature compute |
| src/modules/tensor/cpu/kernel/glitch.cpp | Changed alignedLength divisor/multiplier from 2 to 10 without explanation |
| src/modules/tensor/cpu/kernel/flip.cpp | Adjusted srcPtrTemp pointer arithmetic by subtracting 2 without clarifying constant |
| src/modules/tensor/cpu/kernel/color_temperature.cpp | Added rpp_pixel_check_0to1 calls after each SIMD compute for F32 and F16 |
Comments suppressed due to low confidence (2)
utilities/test_suite/HOST/runImageTests.py:336
- Removing the default else branch leaves
srcPath1andsrcPath2uninitialized for non-lens_correction cases. Reintroduce a fallback assignment toinFilePath1/inFilePath2or handle all branches explicitly.
for layout in range(3):
utilities/test_suite/HIP/runImageTests.py:349
- Similar to the HOST script, removing the default else means
srcPath1/srcPath2may be undefined when the case isn't lens_correction. Add a fallback assignment.
for layout in range(3):
| @@ -370,7 +370,7 @@ RppStatus glitch_f32_f32_host_tensor(Rpp32f *srcPtr, | |||
| { | |||
| Rpp32f *dstPtrRow; | |||
| dstPtrRow = dstPtrChannel; | |||
| Rpp32u alignedLength = (((roi.xywhROI.roiWidth)/ 2) * 2) - 2; | |||
| Rpp32u alignedLength = (((roi.xywhROI.roiWidth)/ 10) * 10) - 10; | |||
There was a problem hiding this comment.
[nitpick] The magic number 10 in alignment logic is unexplained. Consider defining a named constant or adding a comment to clarify why width is aligned to multiples of 10.
| Rpp32u alignedLength = (((roi.xywhROI.roiWidth)/ 10) * 10) - 10; | |
| // Align width to multiples of ALIGNMENT_FACTOR for optimized processing | |
| const Rpp32u ALIGNMENT_FACTOR = 10; | |
| Rpp32u alignedLength = (((roi.xywhROI.roiWidth) / ALIGNMENT_FACTOR) * ALIGNMENT_FACTOR) - ALIGNMENT_FACTOR; |
Copilot uses AI. Check for mistakes.
| @@ -424,7 +424,7 @@ RppStatus flip_f32_f32_host_tensor(Rpp32f *srcPtr, | |||
| dstPtrTempG += vectorIncrementPerChannel; | |||
| dstPtrTempB += vectorIncrementPerChannel; | |||
| } | |||
| srcPtrTemp += hFlipFactor; | |||
| srcPtrTemp += hFlipFactor - 2; | |||
There was a problem hiding this comment.
[nitpick] Subtracting 2 in the pointer increment is a magic number without explanation. Introduce a named constant or comment to describe why two elements are skipped.
| srcPtrTemp += hFlipFactor - 2; | |
| // Skip two elements to align with the processing logic for RGB channels. | |
| constexpr int ELEMENTS_TO_SKIP = 2; | |
| srcPtrTemp += hFlipFactor - ELEMENTS_TO_SKIP; |
Copilot uses AI. Check for mistakes.
| @@ -424,7 +424,7 @@ RppStatus flip_f32_f32_host_tensor(Rpp32f *srcPtr, | |||
| dstPtrTempG += vectorIncrementPerChannel; | |||
| dstPtrTempB += vectorIncrementPerChannel; | |||
| } | |||
There was a problem hiding this comment.
what is this -2 corresponds to
There was a problem hiding this comment.
The -2 adjusts the srcPtrTemp position after the SIMD loop to correctly align it for the remaining scalar processing, especially during horizontal flip. It ensures we don’t skip or misread pixels after vector loads.
There was a problem hiding this comment.
I see that we keep adding lots of binary images for reference output and it keeps growing disk space as we add more and more Ops. We need to consider auto generating using reference code instead, which can be done while running the tests and delete at the end
The PR adds necessary changes to pass internal QA testing support for F32 bit depth for next set of 8 functions - Remap, Flip, Glitch, Ricap, Non linear blend, Gridmask, Color to greyscale, Color temperature.