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

Convert type_utils.py to use arraylias #316

Conversation

DanPuzzuoli
Copy link
Collaborator

@DanPuzzuoli DanPuzzuoli commented Feb 3, 2024

Summary

This PR deletes type_utils.py; most functionality is no longer needed, and the two functions that were kept are moved to models/models_utils.py.

Details and comments

Details of what happened to the contents of type_utils.py:

  • The functions to_array, to_csr, to_BCOO, and to_numeric_matrix_type have been deleted, along with their tests.
    • A few tests throughout the package that were still using to_array unnecessarily have been changed.
    • ScipySparseVectorizedLindbladCollection in operator_collections.py still had a final use of to_csr to remove. I modified the vectorized lindblad classes to no longer need this function, and ended up translating a test case for to_csr handling of qutip Qobj types into a test case for unp.asarray.
  • The StateTypeConverter, along with its tests, have been completely deleted.
    • This class was originally written to handle a general type translation problem, but we didn't end up needing/using it, and in the end it was only used in a single location solvers/scipy_solve_ivp.py.
    • In light of the above, I've deleted the class and added a couple of much-simpler helper functions directly to solvers/scipy_solve_ivp.py.
  • The last remaining functions, vec_commutator and vec_dissipator, are only used in models/operator_collections.py. As such I've move type_utils.py -> models/models_utils.py.

Other changes:

  • Modified test_jax_transformations.py to no longer use Array.
  • All of the arraylias changes have led to slight modifications to how lists of CSR matrices are stored (internally, models store them as numpy object arrays with csr_matrix entries). When returned to a user, they used to return as a list of csr_matrix entries, but now they just return the numpy object array. As such I've had to modify code in a few places that expect a list. These are very minor and sprinkled throughout the package.

Test command (now we can just run all tests! :) ):

python -m unittest discover test

The whole repo should now also pass linting.

@DanPuzzuoli DanPuzzuoli force-pushed the arraylias-integration-branch branch from 1e0947f to a0a262f Compare February 5, 2024 22:35
@qiskit-community qiskit-community deleted a comment from mergify bot Feb 5, 2024
@DanPuzzuoli DanPuzzuoli marked this pull request as ready for review February 5, 2024 23:01
Copy link
Contributor

@to24toro to24toro left a comment

Choose a reason for hiding this comment

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

Thank you. I think the code becomes now very sophisticated.
I only request a great minor change.

Copy link
Contributor

@to24toro to24toro left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM!

@DanPuzzuoli DanPuzzuoli merged commit d7a28d8 into qiskit-community:arraylias-integration-branch Feb 8, 2024
2 checks passed
DanPuzzuoli added a commit that referenced this pull request Feb 20, 2024
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.

2 participants