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

#70: pull in pressiolinalg pypi package #126

Conversation

cwschilly
Copy link
Collaborator

@cwschilly cwschilly commented Feb 2, 2024

Fixes #70

Related to pressio-linalg PR 8


Changes

pressio-linalg is now a required dependency.

  • If you pass an MPI communicator to any of the functions, they will run in parallel. Otherwise, the library defaults to running serial.

Usage

import pressiolinalg.linalg [as pla]

Note that there is no hyphen for the import.


Current API

pla.max(a, axis=None, comm=None)
pla.min(a, axis=None, comm=None)
pla.mean(a, axis=None, dtype=None, comm=None)
pla.std(a, axis=None, dtype=None, ddof=0, comm=None)
pla.product(flag1, flag2, alpha, A, B, beta, C, comm=None)
pla.thin_svd(snapshots, comm=None, method='auto')

These are all subject to change and will be documented in this issue.

@cwschilly cwschilly marked this pull request as draft February 2, 2024 18:19
@cwschilly cwschilly self-assigned this Feb 2, 2024
@eparish1 eparish1 mentioned this pull request Feb 8, 2024
@cwschilly cwschilly force-pushed the 70-linear-algebra-libraries-mpi4py-light-library-and-pressio-tools-full-library branch from cd4338c to d80ecc5 Compare February 12, 2024 19:41
@cwschilly cwschilly marked this pull request as ready for review February 12, 2024 21:35
@cwschilly
Copy link
Collaborator Author

cwschilly commented Feb 12, 2024

@fnrizzi @eparish1 This PR is ready for review.

I added a unit test for the new package, but it will likey become unnecessary once we begin to incorporate the package into source code (since existing tests would then catch any problems).

pressio-linalg also has its own testing suite already.

That being said, should I keep the test in or drop it from the PR?

@fnrizzi
Copy link
Member

fnrizzi commented Feb 13, 2024

That being said, should I keep the test in or drop it from the PR?

the linalg library should have its own testsuite. since that is the case, we should remove tests related to it from this repo.
The romtools repo should only use the linalg package, so just assuming it works.

@fnrizzi
Copy link
Member

fnrizzi commented Feb 13, 2024

@cwschilly can you also please replace in romtools all functions to call the linalg library functions?

@eparish1
Copy link
Contributor

@cwschilly @fnrizzi I agree about Francesco's comments on the tests. But this looks good. I'm fine to replace the calls to linalg in this PR or another one, whatever is easiest.

@cwschilly
Copy link
Collaborator Author

@fnrizzi @eparish1 I've added in pla where applicable--there's a call to np.max in the Scaler that I couldn't update yet since the pla.max only works on rank-1 arrays.

@fnrizzi
Copy link
Member

fnrizzi commented Feb 13, 2024

@fnrizzi @eparish1 I've added in pla where applicable--there's a call to np.max in the Scaler that I couldn't update yet since the pla.max only works on rank-1 arrays.

please add this to linalg then

@cwschilly
Copy link
Collaborator Author

@fnrizzi @eparish1 All calls to np.max, np.min and SVD (method of snapshots) have been replaced

Copy link
Contributor

@eparish1 eparish1 left a comment

Choose a reason for hiding this comment

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

@cwschilly This is wonderful, thanks for doing all of this. This looks great by me. @fnrizzi good by you?

@eparish1
Copy link
Contributor

@cwschilly @fnrizzi you both are tracking where things are at here better than me. Could one of you do the merge when things are ready? Thanks!

@fnrizzi fnrizzi force-pushed the 70-linear-algebra-libraries-mpi4py-light-library-and-pressio-tools-full-library branch from 60ca530 to eb4ea3b Compare March 4, 2024 08:26
@cwschilly
Copy link
Collaborator Author

@fnrizzi @eparish1 This PR should be ready to go now

@fnrizzi fnrizzi merged commit 31146d0 into develop Mar 4, 2024
6 checks passed
@cwschilly cwschilly deleted the 70-linear-algebra-libraries-mpi4py-light-library-and-pressio-tools-full-library branch March 4, 2024 18:27
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.

Linear algebra libraries: mpi4py light library and pressio-tools full library.
3 participants