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

ENH: update backend tests #552

Merged
merged 10 commits into from
Feb 9, 2021
Merged

ENH: update backend tests #552

merged 10 commits into from
Feb 9, 2021

Conversation

samir-nasibli
Copy link
Contributor

@samir-nasibli samir-nasibli commented Jan 31, 2021 •
edited
Loading

Description

  • redesigned backend testing

TODO

@samir-nasibli samir-nasibli added the in progress Please do not merge. Work is in progress. label Jan 31, 2021
@samir-nasibli samir-nasibli removed the in progress Please do not merge. Work is in progress. label Feb 1, 2021
@samir-nasibli
Copy link
Contributor Author

samir-nasibli commented Feb 1, 2021 •
edited
Loading

@shssf CI old fails seem are not related with this PR changes.
All needed changes for this PR logic were added and discussed.
We can merge it as is. On other PR I will try to build and run this tests on new azure-pipelines job.

@samir-nasibli samir-nasibli mentioned this pull request Feb 2, 2021
Merged
@samir-nasibli samir-nasibli mentioned this pull request Feb 3, 2021
Merged
@samir-nasibli samir-nasibli requested a review from shssf February 4, 2021 01:32
@shssf shssf requested a review from densmirn February 8, 2021 22:04
Alexander-Makaryev
Alexander-Makaryev reviewed Feb 9, 2021
View reviewed changes
dpnp/backend/tests/test_random.cpp
}
for (size_t i = 0; i < size; ++i)
{
EXPECT_NEAR(result1[i], result2[i], 0.004);
Copy link
Contributor

Choose a reason for hiding this comment

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

why you are expecting close but not the same values?
looks like values should be the same if you are using the same seed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is common case for checking floating point values. But you are right it is better check the same values here for the same device calculation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Alexander-Makaryev this changes will be done on refactoring this common tests on other PR. This tests case will be generalized for most random distribution functions. Needed changes will be done there. I am leaving this PR as is.

@samir-nasibli
Copy link
Contributor Author

samir-nasibli commented Feb 9, 2021 •
edited
Loading

@Alexander-Makaryev @shssf Most works for this PR are done. This PR was for restructuring generally backend tests. Other comment on #552 (comment)
I am leaving this PR as is.

Alexander-Makaryev
Alexander-Makaryev approved these changes Feb 9, 2021
View reviewed changes
Copy link
Contributor

@Alexander-Makaryev Alexander-Makaryev left a comment

Choose a reason for hiding this comment

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

🔢

@samir-nasibli samir-nasibli merged commit 0774ab6 into master Feb 9, 2021
@samir-nasibli samir-nasibli deleted the samir-nasibli/enh/update_backend_tests branch February 9, 2021 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shssf shssf shssf approved these changes

@Alexander-Makaryev Alexander-Makaryev Alexander-Makaryev approved these changes

@densmirn densmirn Awaiting requested review from densmirn

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@samir-nasibli @shssf @Alexander-Makaryev