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

Update Ad parsing framework to allow for scipy sparse arrays in addition to sparse matrices #1253

Open
keileg opened this issue Oct 31, 2024 · 2 comments
Labels
user group Issue to be worked on in the internal user group.

Comments

@keileg
Copy link
Contributor

keileg commented Oct 31, 2024

Scipy is recommending migration to the new sparse array format, see note here. PorePy will gradually migrate to the new style, but to make that possible, the Ad parsing machinery must be updated. This can be implemented in the following steps:

  1. In all special methods that define mathematical operations (__add__ etc.) in the AdArray class, cases that currently allow for sps.spmatrix should also allow for sps.sparray.
  2. In test_forward_mode, some tests should be modified to cover sparse arrays instead of, or in addition to, sparse matrices. What to replace and what to duplicate must be decided.
  3. In test_operators, the test test_arithmetic_operations_on_ad_objects must be expanded to cover sparse arrays in addition to matrices. In this case, both scipy sparse formats must be kept, since we will need to support both options for the foreseeable future. Note however that we can use exactly the same setup for the two formats, the point is simply to ensure the Ad evaluation framework can deal with both cases.
@keileg keileg added the user group Issue to be worked on in the internal user group. label Oct 31, 2024
@IvarStefansson
Copy link
Contributor

Your comment on deciding between replacement and temporary duplication lead me to https://typing.readthedocs.io/en/latest/spec/directives.html#deprecated. Should we consider using these?

@keileg
Copy link
Contributor Author

keileg commented Nov 1, 2024

As a general comment yes. Not sure how relevant it is for this issue, which mainly considers changes to be made deep inside functions, but that is a different question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
user group Issue to be worked on in the internal user group.
Projects
None yet
Development

No branches or pull requests

2 participants