Skip to content
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.

Sign up for GitHub

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

Jump to bottom

1D test for Reduce layer #25101

Open
wants to merge 6 commits into
base: 5.x
Choose a base branch
from
Open

1D test for Reduce layer #25101

wants to merge 6 commits into from

Conversation

Abdurrahheem
Copy link
Contributor

This PR introduces test for Reduce layer to test its functionality for 1D arrays

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

@Abdurrahheem Abdurrahheem added this to the 5.0 milestone Feb 26, 2024
@Abdurrahheem
Copy link
Contributor Author

Abdurrahheem commented Feb 26, 2024 •
edited

@fengyuentau. I get strange result for log_sum reduce operation. Expected result should be 0 for 1D vector of 1's. The actual result differs from that. Could you please take a look on it?

@asmorkalov asmorkalov added the pr: reproducer Reproduces some bug, not expected to be merged label Feb 26, 2024
@Abdurrahheem
Copy link
Contributor Author

This #25103 issue is also relevant

@Abdurrahheem
Copy link
Contributor Author

@fengyuentau

@Abdurrahheem
Copy link
Contributor Author

@dkurt could you please take a look?

@dkurt dkurt marked this pull request as ready for review March 19, 2024 05:53
fengyuentau
fengyuentau reviewed Mar 22, 2024
View reviewed changes
modules/dnn/test/test_layers_1d.cpp Outdated
Comment on lines 320 to 321
std::make_tuple(0, std::vector<int>{0}),
std::make_tuple(1, std::vector<int>{1}),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you define reduction operation on 0d / 1d tensor? Any reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formally there is not any. This test is to check if layer fails on singleton matrices

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some in onnxruntime: https://github.com/microsoft/onnxruntime/blob/4a196d15940b0f328735c888e2e861d67602ffcf/onnxruntime/test/providers/cpu/reduction/reduction_ops_test.cc#L326 (Search for 0D).

@Abdurrahheem Abdurrahheem force-pushed the ash/1D-reduce-test branch 2 times, most recently from ad5b331 to 6d8ce90 Compare April 3, 2024 08:26
@Abdurrahheem
Copy link
Contributor Author

@dkurt is there anything else to do ?

@Abdurrahheem Abdurrahheem force-pushed the ash/1D-reduce-test branch 2 times, most recently from 756f396 to b29acce Compare April 8, 2024 08:23
opencv-alalek
opencv-alalek reviewed Apr 8, 2024
View reviewed changes
modules/dnn/test/test_layers_1d.cpp
cv::Mat input(input_shape.size(), input_shape.data(), CV_32F, 1.0);
cv::randn(input, 0.0, 1.0);

float out_value = reduceOperation(input, reduce_operation);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do still we have ONE output value for matrix?

Don't write WEAK tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I do not understand you. What could be output for 1D(or 0D) matrix under reduction operation other than one output value? What do you mean by output value?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in case of [4, 4] input and axis=1 output is [4, 1], so it also can be tested

@asmorkalov asmorkalov removed the pr: reproducer Reproduces some bug, not expected to be merged label Apr 23, 2024
@Abdurrahheem
Copy link
Contributor Author

Abdurrahheem commented May 15, 2024 •
edited

@opencv-alalek added what you asked. Anything else?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fengyuentau fengyuentau fengyuentau left review comments

@dkurt dkurt dkurt left review comments

@opencv-alalek opencv-alalek opencv-alalek left review comments

@asmorkalov asmorkalov Awaiting requested review from asmorkalov

Assignees
No one assigned
Labels
category: dnn
Projects
None yet
Milestone
5.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Abdurrahheem @fengyuentau @dkurt @opencv-alalek @asmorkalov