-
Notifications
You must be signed in to change notification settings - Fork 44
Test suite - Add QA pass/fail tests for F32 bit depth #549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test suite - Add QA pass/fail tests for F32 bit depth #549
Conversation
RPP F32 QA support for 7 augmentations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces additional QA tests for F32 bit depth processing along with range‐check validations for tensor functions. Key changes include:
- Adding new validations for data type and layout consistency before processing in several host and GPU kernels.
- Inserting boundary checks for F32 values (using rpp_pixel_check_0to1) across multiple tensor processing functions.
- Updating API definitions and simd load/store helper functions to support the new checks.
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/modules/tensor/rppt_tensor_geometric_augmentations.cpp | Added validations for matching data types and valid layouts prior to crop/patch operations. |
src/include/common/cpu/rpp_cpu_simd_load_store.hpp | Updated rpp_pixel_check_0to1 functions to modify vectors in place rather than returning clamped values. |
src/modules/tensor/cpu/kernel/contrast.cpp | Inserted boundary checks for F32 values to ensure pixel intensity remains in the expected [0, 255] range. |
Comments suppressed due to low confidence (2)
src/include/common/cpu/rpp_cpu_simd_load_store.hpp:275
- The function signature of rpp_pixel_check_0to1 has been changed to modify the vector in place rather than returning a value; please ensure that all call sites have been updated accordingly.
inline __m256 rpp_pixel_check_0to1_avx(__m256 p)
src/modules/tensor/cpu/kernel/contrast.cpp:310
- [nitpick] The addition of boundary checks for F32 pixels ensures values remain within acceptable limits; verify that similar checks in other kernels are consistent and do not introduce performance regressions.
rpp_pixel_check_0to1(p, 3);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request adds F32 bit depth QA test support by introducing range validations and error checking both at the API level and inside the host/GPU kernels. Key changes include adding explicit error returns for mismatched dataType and unsupported layouts, inserting boundary check calls (rpp_pixel_check_0to1) after pixel processing, and updating SIMD load/store functions to work with the revised boundary enforcement.
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/modules/tensor/rppt_tensor_geometric_augmentations.cpp | Added validation checks on data types and layouts in crop functions. |
src/modules/tensor/rppt_tensor_color_augmentations.cpp | Introduced similar dataType/layout constraints and boundary checks. |
src/modules/tensor/cpu/kernel/threshold.cpp | Replaced legacy rpp_pixel_check_0to1_avx calls with the updated rpp_pixel_check_0to1 function for F32. |
src/include/common/cpu/rpp_cpu_simd_load_store.hpp | Updated the rpp_pixel_check_0to1 functions’ signatures to operate on vector arrays and return void. |
... | Several kernel files (phase.cpp, brightness.cpp, etc.) have analogous boundary check insertions and API updates. |
Comments suppressed due to low confidence (1)
src/modules/tensor/cpu/kernel/threshold.cpp:42
- The legacy rpp_pixel_check_0to1_avx call has been replaced by a new boundary check call; please verify that the new rpp_pixel_check_0to1 function correctly enforces the [0,1] range across all SIMD vector lengths.
p[0] = _mm256_blendv_ps(avx_p0, avx_p1, _mm256_and_ps(_mm256_and_ps(pChannelCheck[0], pChannelCheck[1]), pChannelCheck[2]));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added few comments
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #549 +/- ##
========================================
Coverage 85.78% 85.78%
========================================
Files 185 185
Lines 78985 79060 +75
========================================
+ Hits 67751 67819 +68
- Misses 11234 11241 +7
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we have discussed, please add the pixel check only in the functions where it is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@r-abishek there was a comment which is not resolved
* Modified SIMD print functions to use union * remove redundant unions in print functions * removed pixel checks * remove pixel check in threshold * resolve review comments
@rrawther All comments resolved now |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
The PR adds internal QA testing support for F32 bit depth for next set of functions.