Skip to content

Introduce errstate configuration to control cuSOLVER devInfo and cuBLAS infoArray checks#2437

Merged
asi1024 merged 10 commits intocupy:masterfrom
hvy:linalg-env-dev-info
Nov 14, 2019
Merged

Introduce errstate configuration to control cuSOLVER devInfo and cuBLAS infoArray checks#2437
asi1024 merged 10 commits intocupy:masterfrom
hvy:linalg-env-dev-info

Conversation

@hvy
Copy link
Copy Markdown
Member

@hvy hvy commented Aug 30, 2019

Fixes #2414.

Please review the design.

This PR introduces an environment variable to control (turn on/off) the devInfo check after cuSOLVER routine calls. After discussions in the above issue, the default behavior was decided to not check the devInfo. This PR is labaled as no-compat since it will silence errors in existing code and break NumPy compatibility.

I'm open for discussion about the design and will continue adding tests and documentation once that looks okay.

TODO:

  • Implement
  • Test
  • Document

@hvy hvy added cat:feature New features/APIs no-compat Disrupts backward compatibility labels Aug 30, 2019
@hvy hvy force-pushed the linalg-env-dev-info branch from 357cbff to 232c58f Compare August 30, 2019 06:34
@hvy
Copy link
Copy Markdown
Member Author

hvy commented Sep 27, 2019

There are still opinions about unifying the default behavior to actually sync and check the devInfo (see #2414). Would be nice to agree on either before merging new PRs (e.g. #2495).

@hvy hvy changed the title [WIP] Introduce environment variable to control cuSOLVER devInfo check Introduce environment variable to control cuSOLVER devInfo check Oct 7, 2019
@hvy
Copy link
Copy Markdown
Member Author

hvy commented Oct 7, 2019

Added tests for routines where I could come up with negative cases (e.g. singular inputs). I'm not sure how to test the rest of them since the expected negative results are not necessarily obvious nor consistent with NumPy. PTAL.

@asi1024
Copy link
Copy Markdown
Member

asi1024 commented Nov 5, 2019

Could you fix to use errstate introduced in #2535?

@hvy hvy force-pushed the linalg-env-dev-info branch from 1d002e2 to 9b68a66 Compare November 11, 2019 06:58

@contextlib.contextmanager
def errstate(*, divide=None, over=None, under=None, invalid=None):
def errstate(*, divide=None, over=None, under=None, invalid=None, linalg=None):
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I guess we should document all these arguments in all these functions, in a separate PR.

@hvy
Copy link
Copy Markdown
Member Author

hvy commented Nov 11, 2019

Updated to use errstate. PTAL.

@hvy
Copy link
Copy Markdown
Member Author

hvy commented Nov 11, 2019

Fixed _batched_inv as well (which actually calls a cuBLAS routine and not a cuSOLVER one). PTAl.

@hvy
Copy link
Copy Markdown
Member Author

hvy commented Nov 11, 2019

Let me update the title of this PR reflect the new changes.

@hvy hvy changed the title Introduce environment variable to control cuSOLVER devInfo check Introduce errstate configuration to control cuSOLVER devInfo and cuBLAS infoArray checks Nov 11, 2019
err = True
err_detail += '\tmatrix[{}]: matrix is singular.\n'.format(i)
if err:
raise RuntimeError('matrix inversion failed at getrf.\n' + err_detail)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Note that this PR changes this error type to numpy.linalg.LinAlgError.

@asi1024
Copy link
Copy Markdown
Member

asi1024 commented Nov 13, 2019

LGTM. Jenkins, test this please.

@pfn-ci-bot
Copy link
Copy Markdown
Collaborator

Successfully created a job for commit fd5cab9:

@asi1024 asi1024 added this to the v7.0.0 milestone Nov 13, 2019
@chainer-ci
Copy link
Copy Markdown
Member

Jenkins CI test (for commit fd5cab9, target branch master) failed with status FAILURE.

@asi1024
Copy link
Copy Markdown
Member

asi1024 commented Nov 13, 2019

@asi1024
Copy link
Copy Markdown
Member

asi1024 commented Nov 14, 2019

Jenkins, test this please.

@pfn-ci-bot
Copy link
Copy Markdown
Collaborator

Successfully created a job for commit fd5cab9:

@chainer-ci
Copy link
Copy Markdown
Member

Jenkins CI test (for commit fd5cab9, target branch master) succeeded!

@asi1024
Copy link
Copy Markdown
Member

asi1024 commented Nov 14, 2019

LGTM.

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

Labels

cat:feature New features/APIs no-compat Disrupts backward compatibility

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing error checks for cuSOLVER calls

4 participants