Skip to content

5.x merge 4.x: vectorized normDiff#27068

Merged
asmorkalov merged 8 commits intoopencv:5.xfrom
fengyuentau:5x-merge-4x/core/normDiff-simd
Mar 24, 2025
Merged

5.x merge 4.x: vectorized normDiff#27068
asmorkalov merged 8 commits intoopencv:5.xfrom
fengyuentau:5x-merge-4x/core/normDiff-simd

Conversation

@fengyuentau
Copy link
Copy Markdown
Member

@fengyuentau fengyuentau commented Mar 14, 2025

Merge with opencv/opencv_extra#1243

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@fengyuentau fengyuentau requested a review from asmorkalov March 14, 2025 10:40
@asmorkalov asmorkalov self-assigned this Mar 14, 2025
@asmorkalov asmorkalov added this to the 5.0-release milestone Mar 14, 2025
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Mar 14, 2025

Please take a look on the Python test failure:


======================================================================
FAIL: test_norm_for_two_arrays (test_norm.norm_test)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/opencv-cn/GHA-OCV-1/_work/opencv/opencv/opencv/modules/python/test/test_norm.py", line 134, in test_norm_for_two_arrays
    norm_name[norm_type]
AssertionError: 55.0 != 4294967241.0 within 2 places (4294967186.0 difference) : Arrays [37] [92] of type int32 and norm inf

@asmorkalov asmorkalov self-requested a review March 14, 2025 12:48
@fengyuentau
Copy link
Copy Markdown
Member Author

fengyuentau commented Mar 14, 2025

I am afraid that we need to backport the fix 78a84e3 to 4.x.

@fengyuentau
Copy link
Copy Markdown
Member Author

fengyuentau commented Mar 14, 2025

Also I noticed that 5.x changes the return type of norm inf from int to unsigned.

5.x:

opencv/modules/core/src/norm.simd.hpp

Line 710 in 4919cda

CV_DEF_NORM_ALL(32s, int, unsigned, double, double)

4.x:

opencv/modules/core/src/norm.simd.hpp

Line 1358 in 14396b8

CV_DEF_NORM_ALL(32s, int, int, double, double)

Corresponding kernel needs to be updated.

@fengyuentau
Copy link
Copy Markdown
Member Author

fengyuentau commented Mar 16, 2025 •
edited
Loading

It seems that different compiler handles normL1 of 32s differently. I cannot make it work.

Even though I tried generating perf sanity data from the latest 5.x (w/o this patch), it is just different on different compiler.

For example, clang and msvc's products get result 438524980217699 on the perf test case Size_MatType_NormType_norm2.norm2/31, where GetParam() = (640x480, 32SC1, NORM_L1), while gcc's get result 274616143300451.

@fengyuentau

This comment was marked as duplicate.

Sign in to view
@fengyuentau
Copy link
Copy Markdown
Member Author

fengyuentau commented Mar 17, 2025

Accuracy issue on 32s kernels: #27080.

fengyuentau
fengyuentau commented Mar 18, 2025
View reviewed changes
fengyuentau added a commit to fengyuentau/opencv that referenced this pull request Mar 18, 2025
@fengyuentau
ported some changes from opencv#27068
058ed08
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Mar 20, 2025

There are test failures on Mac:

[ RUN      ] Size_MatType_NormType_norm2_mask.norm2_mask/31, where GetParam() = (640x480, 32SC1, NORM_L1)
 Expected: 
[274616143300451]
 Actual:
[438524980217699]
/Users/opencv-cn/GHA-OCV-1/_work/opencv/opencv/opencv/modules/ts/src/ts_perf.cpp:589: Failure
Failed
  Relative difference (163908836917248 of 26311498813.061943 allowed) between argument "n" and expected value is greater than 1.0000000000000001e-05 in 1 points
params    = (640x480, 32SC1, NORM_L1)
termination reason:  reached maximum number of iterations
bytesIn   =    2764800
bytesOut  =          0
samples   =          1
outliers  =          0
frequency = 1000000000
min       =     594942 = 0.59ms
median    =     594942 = 0.59ms
gmean     =     594942 = 0.59ms
gstddev   = 0.00000000 = 0.00ms for 97% dispersion interval
mean      =     594942 = 0.59ms
stddev    =          0 = 0.00ms
[  FAILED  ] Size_MatType_NormType_norm2_mask.norm2_mask/31, where GetParam() = (640x480, 32SC1, NORM_L1) (5 ms)

@fengyuentau
Copy link
Copy Markdown
Member Author

fengyuentau commented Mar 21, 2025

There are test failures on Mac:

[ RUN      ] Size_MatType_NormType_norm2_mask.norm2_mask/31, where GetParam() = (640x480, 32SC1, NORM_L1)
 Expected: 
[274616143300451]
 Actual:
[438524980217699]
/Users/opencv-cn/GHA-OCV-1/_work/opencv/opencv/opencv/modules/ts/src/ts_perf.cpp:589: Failure
Failed
  Relative difference (163908836917248 of 26311498813.061943 allowed) between argument "n" and expected value is greater than 1.0000000000000001e-05 in 1 points
params    = (640x480, 32SC1, NORM_L1)
termination reason:  reached maximum number of iterations
bytesIn   =    2764800
bytesOut  =          0
samples   =          1
outliers  =          0
frequency = 1000000000
min       =     594942 = 0.59ms
median    =     594942 = 0.59ms
gmean     =     594942 = 0.59ms
gstddev   = 0.00000000 = 0.00ms for 97% dispersion interval
mean      =     594942 = 0.59ms
stddev    =          0 = 0.00ms
[  FAILED  ] Size_MatType_NormType_norm2_mask.norm2_mask/31, where GetParam() = (640x480, 32SC1, NORM_L1) (5 ms)

Fixed.

@asmorkalov asmorkalov mentioned this pull request Mar 23, 2025
Closed
@asmorkalov asmorkalov merged commit 98cbe0a into opencv:5.x Mar 24, 2025
49 of 51 checks passed
@fengyuentau fengyuentau deleted the 5x-merge-4x/core/normDiff-simd branch March 24, 2025 06:46
@fengyuentau fengyuentau mentioned this pull request Apr 27, 2025
Merged
asmorkalov pushed a commit that referenced this pull request Apr 28, 2025
@fengyuentau
Merge pull request #27261 from fengyuentau:5.x-merge-4.x-norm-hal_rvv
3bfc408 
5.x merge 4.x: merge changes of norm and norm_diff in hal rvv from 4.x #27261

Merge with opencv/opencv_extra#1251

No related changes in contrib

#26991 from fengyuentau:4x/core/norm2hal_rvv
#27045 from fengyuentau:4x/hal_rvv/normDiff

Previous "Merge 4.x" on norm_diff vectorization: #27068
@fengyuentau fengyuentau mentioned this pull request Apr 29, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asmorkalov asmorkalov asmorkalov approved these changes

Assignees

@asmorkalov asmorkalov

Labels

optimization

Projects

None yet

Milestone

5.0-release

Development

Successfully merging this pull request may close these issues.

2 participants

@fengyuentau @asmorkalov