Skip to content

Add matrix_power specification #112

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

Merged
merged 9 commits into from
May 12, 2021
Merged

Add matrix_power specification #112

merged 9 commits into from
May 12, 2021

Conversation

kgryte
Copy link
Contributor

@kgryte kgryte commented Jan 14, 2021

This PR

  • specifies an interface for raising a square matrix to an integer power.
  • is derived from comparing signatures across array libraries.

Notes

  • The output array dtype for NumPy depends on both the input array and whether n is negative. If negative, then the result is floating-point. If non-negative, the result has the same dtype as the input. For CuPy, only float32 and float64 is allowed. In general, we recommend restricting linalg functions to floating-point dtypes. That practice is followed here.

  • Following Torch and maintaining consistency with other linalg interfaces operating in square matrices, a stack of square matrices is accepted.

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

For CuPy, only float32 and float64 is allowed. In general, we restrict linalg functions to floating-point dtypes. That practice is followed here.

Only specifying float32/64 sounds right, however the phrasing is a bit off. I'd rather add a note saying that the behaviour is only specified for these dtypes, and that for other dtypes implementation may differ - they will either cast or raise for some or all values of n.

@kgryte
Copy link
Contributor Author

kgryte commented Jan 28, 2021

Re: dtypes. We'd need to add that note more generally at the top of the linalg spec, as this is generally true for most linalg specs, given their current support being "limited" to floating-point dtypes. The restriction to floating-point dtypes was discussed a couple times during meetings and found its support there.

@rgommers rgommers added the API extension Adds new functions or objects to the API. label Mar 20, 2021
@kgryte
Copy link
Contributor Author

kgryte commented Mar 24, 2021

@rgommers re: dtypes. We could change the language from must to should. As in, we could recommend floating-point input, but allow implementations to accept, e.g., integer dtypes.

In this particular case, I would, however, advocate for always returning a floating-point dtype.

@rgommers rgommers force-pushed the main branch 3 times, most recently from 0607525 to 138e963 Compare April 19, 2021 20:25
@kgryte
Copy link
Contributor Author

kgryte commented May 12, 2021

This PR has been open for some time without further comment and has been discussed/approved during meetings. Will merge, and we can submit follow-up PRs to resolve any issues/concerns which may arise.

@kgryte kgryte merged commit 30b0391 into main May 12, 2021
@kgryte kgryte deleted the matrix-power branch May 12, 2021 05:14
@kgryte
Copy link
Contributor Author

kgryte commented May 12, 2021

For reference, inclusion of this API in the standard was discussed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API extension Adds new functions or objects to the API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants