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.

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

Polyval test Assertion Error Fix #67

Merged
merged 6 commits into from
Nov 7, 2024

Conversation

shbiswas834
Copy link

This PR does the following:

  • Tolerance fix for polyval test associated this Jira SWDEV-471852
    tests/cupy_tests/lib_tests - TestPolyval::test_polyval - AssertionError
  • Reverting PR Use vector norm for allclose comparisons #63 since the UT passes without these changes and with the new tolerance
    • Attached logs has all UTs which call check_func from PR 63

cupy_unit_tests_pr_revert.log

@shbiswas834 shbiswas834 self-assigned this Nov 1, 2024
Copy link

@pnunna93 pnunna93 left a comment

Choose a reason for hiding this comment

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

@shbiswas834 the PR is changing file permissions, please revert them and add condition for rocm as mentioned below.

@@ -694,7 +694,7 @@ def test_polyfit_weighted_diff_types(self, xp, dtype1, dtype2, dtype3):
class TestPolyval(Poly1dTestBase):

@testing.for_all_dtypes()
@testing.numpy_cupy_allclose(rtol={numpy.float16: 1e-2, 'default': 1e-3})
@testing.numpy_cupy_allclose(rtol={numpy.float16: 1e-2, 'default': 2e-3})
Copy link

Choose a reason for hiding this comment

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

This changes tolerance for both rocm and cuda, please add a conditional check for rocm. You can use runtime.is_hip to check the platform.

Copy link
Author

Choose a reason for hiding this comment

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

Hey @pnunna93 Added this check. UT passes
ut_lib.log

Copy link

Choose a reason for hiding this comment

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

Thanks, please fix file permissions and static checks.

@shbiswas834 shbiswas834 force-pushed the shreya/polyval_tolarance branch 2 times, most recently from 4052fa4 to e3dbff1 Compare November 6, 2024 20:19
@shbiswas834 shbiswas834 force-pushed the shreya/polyval_tolarance branch from e3dbff1 to 9c6dff7 Compare November 6, 2024 20:20
Copy link

@pnunna93 pnunna93 left a comment

Choose a reason for hiding this comment

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

LGTM

@pnunna93 pnunna93 merged commit 25b5b33 into rocm6.2_internal_testing Nov 7, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants