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

Current status of pressio-linalg API #2

Open
cwschilly opened this issue Nov 15, 2023 · 7 comments
Open

Current status of pressio-linalg API #2

cwschilly opened this issue Nov 15, 2023 · 7 comments

Comments

@cwschilly
Copy link
Collaborator

cwschilly commented Nov 15, 2023

This issue will document the current API of functions implemented in the repo and compare it to the API of other Python libraries (np.linalg, dask).

function pressio-linalg numpy dask
max of a distributed array max(a, axis=None, comm=None) np.max(a, axis=None, out=None, keepdims=<no value>, initial=<no value>, where=<no value>) dask.array.max(a, axis=None, keepdims=False, split_every=None, out=None)
min of a distributed array min(a, axis=None, comm=None) np.min(a, axis=None, out=None, keepdims=<no value>, initial=<no value>, where=<no value>) dask.array.min(a, axis=None, keepdims=False, split_every=None, out=None)
mean of a distributed array mean(a, axis=None, dtype=None, comm=None) np.mean(a, axis=None, dtype=None, out=None, keepdims=, *, where=) dask.array.mean(a, axis=None, dtype=None, keepdims=False, split_every=None, out=None)
standard deviation std(a, axis=None, dtype=None, ddof=0, comm=None) np.std(a, axis=None, dtype=None, out=None, ddof=0, keepdims=, *, where=) dask.array.std(a, axis=None, dtype=None, keepdims=False, ddof=0, split_every=None, out=None)
matrix product 1 product(flag1, flag2, alpha, A, B, beta, C, comm=None) np.matmul(x1, x2, /, out=None, *, casting='same_kind', order='K', dtype=None, subok=True[, signature, extobj, axes, axis]) dask.array.matmul(x1, x2, /, out=None, *, casting='same_kind', order='K', dtype=None, subok=True[, signature, extobj, axes, axis])
SVD (method of snapshots) 2 thin_svd(M, comm=None, method='auto') np.linalg.svd(a, full_matrices=True, compute_uv=True, hermitian=False) dask.array.linalg.svd(a, coerce_signs=True)

Footnotes

  1. The public-facing name for the matrix product function has not been finalized

  2. The np and dask API are for standard SVD (not using the method of snapshots)

@cwschilly cwschilly changed the title Current status of linalg repo Current status of pressio-linalg API Nov 15, 2023
@fnrizzi
Copy link
Member

fnrizzi commented Nov 15, 2023

what is this:

...(vec, comm)

?

@cwschilly
Copy link
Collaborator Author

cwschilly commented Nov 15, 2023

@fnrizzi The function names in the repo are somewhat long, so I left them out to keep things cleaner. I added a clarifying statement to the original comment. Would you prefer that I add them in? (for example, this would be _basic_min_via_python(vec, comm))

@fnrizzi
Copy link
Member

fnrizzi commented Nov 15, 2023

@cwschilly those are the internal names of the functions, which we do not want to expose.
We need to write above the PUBLIC names of the functions

@cwschilly
Copy link
Collaborator Author

@fnrizzi Okay, I have added in the current public-facing function names

@fnrizzi
Copy link
Member

fnrizzi commented Nov 15, 2023

some suggestions:

  • dist_max -> max : and similar for min, we don't need to specify the dist_ since this implied by the fact that these are functions accepting a communicator
  • A^T B: need to think about this
  • svd_method_of_snapshots: need to think about this too, maybe svd_via_method_of_snapshots. There is no counterpart of this , because effectively this is internally just svd(A^T B) or similar. we callit svd_via_method_of_snapshots just as syntactic sugar

@fnrizzi
Copy link
Member

fnrizzi commented Nov 20, 2023

At_dot_b(A, b, comm)

this we need to change into:

product(flag1, flag2, alpha, A, B, beta, A)

whre flag1 and flag2 indicate whether to transpose or not A and B, and alpoha and betaj are coefficients.
So we would have for example:

product('T', 'N', 1, A, A, 0, C)

to indicate we want C = A^T A

But wait, i need to think about this first to see how all this fits into the other repo

@cwschilly
Copy link
Collaborator Author

@fnrizzi This change has been made to main; I'll update the chart above to reflect the new API

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

No branches or pull requests

2 participants