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

Add w/a for oneMath to fix test oneMKL interfaces action #2360

Merged
merged 5 commits into from
Mar 7, 2025

Conversation

vlad-perevezentsev
Copy link
Collaborator

This PR suggests adding a temporary workaround to a problem in oneMath #642 where exceptions are no longer thrown in lapack namespace for getrf function as expected.

In oneMath develop branch oneapi::mkl::lapack::computation_error is not thrown.
Instead, oneapi::mkl::computation_error from mkl namespace is used so existing catch block mkl_lapack::exception does not handle singular matrix errors.

A workaround has been added to explicitly catch oneapi::mkl::computation_error and update dev_info ensuring that singular matrices are handled correctly.

  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • Have you checked performance impact of proposed changes?
  • Have you added documentation for your changes, if necessary?
  • Have you added your changes to the changelog?

Copy link
Contributor

github-actions bot commented Mar 5, 2025

View rendered docs @ https://intelpython.github.io/dpnp/index.html

@coveralls
Copy link
Collaborator

coveralls commented Mar 5, 2025

Coverage Status

coverage: 71.744% (-0.007%) from 71.751%
when pulling 2ce002d on fix_public_ci_onemath
into acd33e4 on master.

Copy link
Contributor

github-actions bot commented Mar 5, 2025

Array API standard conformance tests for dpnp=0.18.0dev0=py312he4f9c94_32 ran successfully.
Passed: 1006
Failed: 0
Skipped: 17

sycl_free_noexcept(scratchpad, exec_q);
if (ipiv != nullptr)
sycl_free_noexcept(ipiv, exec_q);
throw LinAlgError("The input coefficient matrix is singular.");
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious why is it handled in another way comparing to oneapi::mkl::lapack::computation_error above?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Above we handle all exceptions in oneapi::mkl::lapack::exception class by defining a specific exception using the value of info() method.
In this case if the input matrix is singular, the exception oneapi::mkl::computation_error is thrown.

Also info method is not available in oneapi::mkl::exception: class

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, the main reason is that we can't use gesv_utils::handle_lapack_exc here, because oneapi::mkl::computation_error doesn't expose info() method.

Copy link
Contributor

@antonwolfy antonwolfy left a comment

Choose a reason for hiding this comment

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

Thank you @vlad-perevezentsev for investigating the issue with oneMath tests scope and implementing the workaround.

@vlad-perevezentsev vlad-perevezentsev merged commit 1b0ce60 into master Mar 7, 2025
66 of 67 checks passed
@vlad-perevezentsev vlad-perevezentsev deleted the fix_public_ci_onemath branch March 7, 2025 16:13
github-actions bot added a commit that referenced this pull request Mar 7, 2025
This PR suggests adding a temporary workaround to a problem in oneMath
[#642 ](uxlfoundation/oneMath#642) where
exceptions are no longer thrown in `lapack` namespace for `getrf`
function as expected.

In oneMath develop branch `oneapi::mkl::lapack::computation_error` is
not thrown.
Instead, `oneapi::mkl::computation_error` from `mkl` namespace is used
so existing catch block `mkl_lapack::exception` does not handle singular
matrix errors.

A workaround has been added to explicitly catch
`oneapi::mkl::computation_error` and update `dev_info` ensuring that
singular matrices are handled correctly. 1b0ce60
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