Add sparse modules to update dynamics by arraylias #286

to24toro · 2023-11-07T09:11:11Z

Summary

I added some modules of jax_sparse and numpy_sparse. I wrote the code, looking at the model class and don't know if it is suite for other classes.
In order to make Operator input work, I think it is necessary to add Operator as a new type to dynamics arraylias. Maybe, we need to add list too, but I cannot conclude the necessarily of the list now because it is on going about integrating the model.

Details and comments

DanPuzzuoli · 2023-11-07T15:42:33Z

In order to make Operator input work, I think it is necessary to add Operator as a new type to dynamics arraylias.

Hmm possibly. I do know though that Operator implements __array__, so:

unp.asarray(Operator.from_label('X'))

works, and even

unp.asarray([Operator.from_label('X'), Operator.from_label('Y')])

already works as is. Maybe this is enough?

DanPuzzuoli

Overall looks good - excited for these changes.

Comments:

I made a bunch of suggestions - just making sure the file and function doc strings are full sentences.
Can we get rid of to_numeric_matrix_type by registering a suitable default for asarray instead?
Do we need rmatmul anymore? I think we had set this up to originally so we could dispatch on the second argument of matmul, but we may just be able to use the _numpy_multi_dispatch function with matmul instead.

Beyond this, I guess an issue with scipy sparse is that we store lists of them in numpy arrays, e.g.

np.array([csr_matrix(...), csr_matrix(...)])

so the library inference doesn't work. One solution to this could be to define a new class with a name like SparseMatrixList that just stores the numpy object array of sparse matrices, is registered as a scipy_sparse type, and passes all ufuncs/numpy function calls through to the underlying numpy object array.

DanPuzzuoli · 2023-11-07T15:53:09Z

qiskit_dynamics/arraylias/register_functions/to_numeric_matrix_type.py

+from qiskit_dynamics.type_utils import isinstance_qutip_qobj
+
+
+def register_to_numeric_matrix_type(alias):


I'm wondering now if this is actually necessary - this basically functions as unp.asarray except it has a default. Can we instead just register a default for asarray? I think this should work - is there some intended distinction between asarray and to_numeric_matrix_type that I'm not remembering?

Yes. I'm just considering it, but I cannot decide it because it is used in model files I have not seen yet.

Certainly, I also feel we can integrate it to asarray.

Perhaps for now just remove it and we'll see if it can be replaced effectively with asarray in the model files.

qiskit_dynamics/arraylias/register_functions/to_sparse.py

qiskit_dynamics/arraylias/register_functions/to_dense.py

qiskit_dynamics/arraylias/register_functions/rmatmul.py

qiskit_dynamics/arraylias/register_functions/multiply.py

qiskit_dynamics/arraylias/register_functions/matmul.py

to24toro · 2023-11-08T01:44:50Z

Thanks. It's no need to add "Operator"(https://github.com/Qiskit/qiskit/blob/af643eb09b22fc3833084b3112890b1aebdcbf92/qiskit/quantum_info/operators/operator.py#L120) as you mentioned.

Co-authored-by: Daniel Puzzuoli <[email protected]>

to24toro · 2023-11-08T01:55:47Z

Do we need rmatmul anymore? I think we had set this up to originally so we could dispatch on the second argument of matmul, but we may just be able to use the _numpy_multi_dispatch function with matmul instead.

Yes, it makes sense but I don't think we can use _numpy_multi_dispatch which only judges whether the array type is "numpy" or "jax" not "sparse. We need to add sparse types to _prefer_libs.

to24toro · 2023-11-08T02:16:52Z

Beyond this, I guess an issue with scipy sparse is that we store lists of them in numpy arrays, e.g.
np.array([csr_matrix(...), csr_matrix(...)])
so the library inference doesn't work. One solution to this could be to define a new class with a name like SparseMatrixList that just stores the numpy object array of sparse matrices, is registered as a scipy_sparse type, and passes all ufuncs/numpy function calls through to the underlying numpy object array.

I think the reason we want to add list type to alias is to treat such as List[spase_matrix]. Therefore, it is a good idea.

DanPuzzuoli

Aside from the direct comments, another change I would suggest is to put the register_fallback and register_default functions together in the files (currently the default functions appear at the beginning, and the fallback functions appear at the end).

I believe these suggestions should fix the current issues. There is another problem with tests raising warnings, but I think these are the result of a JAX update possibly. I will see if I can resolve these in a separate PR.

Edit: the docs build warnings should be fixed in #287 .

qiskit_dynamics/arraylias/register_functions/asarray.py

qiskit_dynamics/arraylias/register_functions/to_dense.py

DanPuzzuoli · 2023-11-08T16:42:57Z

qiskit_dynamics/arraylias/register_functions/to_numeric_matrix_type.py

+from qiskit_dynamics.type_utils import isinstance_qutip_qobj
+
+
+def register_to_numeric_matrix_type(alias):


Perhaps for now just remove it and we'll see if it can be replaced effectively with asarray in the model files.

Co-authored-by: Daniel Puzzuoli <[email protected]>

removing import of non-existant register functions

Removing calls of non-existant functions

DanPuzzuoli

This LGTM. The only thing missing is some tests - can you add a few test cases for the new types/functions?

qiskit_dynamics/arraylias/register_functions/asarray.py

DanPuzzuoli

Thanks Kento - the tests look good. I've left a few comments.

The main thing is to add validation for the output type of the dispatched functions. I've commented in a few places, but I think this should be done everywhere.

DanPuzzuoli · 2023-11-16T18:50:40Z

test/dynamics/arraylias/register_functions/test_matmul.py

+        """Test matmul for scipy_sparse."""
+        x = csr_matrix([[1, 0], [0, 1]])
+        y = csr_matrix([[2, 2], [2, 2]])
+        self.assertAllClose(csr_matrix.toarray(unp.matmul(x, y)), [[2, 2], [2, 2]])


validate type of unp.matmul(x, y)

DanPuzzuoli · 2023-11-16T18:50:55Z

test/dynamics/arraylias/register_functions/test_matmul.py

+
+            x = BCOO.fromdense([[1, 0], [0, 1]])
+            y = BCOO.fromdense([[2, 2], [2, 2]])
+            self.assertAllClose(BCOO.todense(unp.matmul(x, y)), [[2, 2], [2, 2]])


same thing here, validate type of unp.matmul(x, y)

DanPuzzuoli · 2023-11-16T18:51:57Z

test/dynamics/arraylias/register_functions/test_matmul.py

+from ...common import QiskitDynamicsTestCase
+
+
+class TestMultiplyFunction(QiskitDynamicsTestCase):


TestMatmulFunction

DanPuzzuoli · 2023-11-16T18:52:16Z

test/dynamics/arraylias/register_functions/test_rmatmul.py

+from ...common import QiskitDynamicsTestCase
+
+
+class TestMultiplyFunction(QiskitDynamicsTestCase):


TestRmatmulFunction

add sparse module

56f5e7a

DanPuzzuoli requested changes Nov 7, 2023

View reviewed changes

to24toro and others added 10 commits November 8, 2023 10:49

Update qiskit_dynamics/arraylias/register_functions/to_sparse.py

1f0f136

Co-authored-by: Daniel Puzzuoli <[email protected]>

Update qiskit_dynamics/arraylias/register_functions/to_sparse.py

a56088f

Co-authored-by: Daniel Puzzuoli <[email protected]>

Update qiskit_dynamics/arraylias/register_functions/rmatmul.py

ab5ec7d

Co-authored-by: Daniel Puzzuoli <[email protected]>

Update qiskit_dynamics/arraylias/register_functions/multiply.py

907a878

Co-authored-by: Daniel Puzzuoli <[email protected]>

Update qiskit_dynamics/arraylias/register_functions/to_dense.py

025454e

Co-authored-by: Daniel Puzzuoli <[email protected]>

Update qiskit_dynamics/arraylias/register_functions/to_dense.py

d9e537d

Co-authored-by: Daniel Puzzuoli <[email protected]>

Update qiskit_dynamics/arraylias/register_functions/matmul.py

6687bb6

Co-authored-by: Daniel Puzzuoli <[email protected]>

Update qiskit_dynamics/arraylias/register_functions/rmatmul.py

41d8fc7

Co-authored-by: Daniel Puzzuoli <[email protected]>

Update qiskit_dynamics/arraylias/register_functions/multiply.py

d075ded

Co-authored-by: Daniel Puzzuoli <[email protected]>

Update qiskit_dynamics/arraylias/register_functions/matmul.py

9fe669e

Co-authored-by: Daniel Puzzuoli <[email protected]>

DanPuzzuoli requested changes Nov 8, 2023

View reviewed changes

to24toro mentioned this pull request Nov 13, 2023

[WIP] Arraylias integration -Model- #282

Closed

to24toro and others added 8 commits November 13, 2023 22:23

Update qiskit_dynamics/arraylias/register_functions/asarray.py

c962a34

Co-authored-by: Daniel Puzzuoli <[email protected]>

Update qiskit_dynamics/arraylias/register_functions/to_dense.py

64cbf56

Co-authored-by: Daniel Puzzuoli <[email protected]>

change the order of the funtion places

8dccad1

remove to_dense to_sparse to_numeric_matrix_type

44c1c4f

lint

ced1dc4

Update alias.py

9fdce08

removing import of non-existant register functions

Merge branch 'main' into arraylias/sparse_module

903b42f

Update alias.py

12ddc08

Removing calls of non-existant functions

DanPuzzuoli requested changes Nov 15, 2023

View reviewed changes

to24toro added 2 commits November 16, 2023 12:54

addtest

f6f109e

remove fallback

8b674fe

to24toro commented Nov 16, 2023

View reviewed changes

qiskit_dynamics/arraylias/register_functions/asarray.py Outdated Show resolved Hide resolved

DanPuzzuoli requested changes Nov 16, 2023

View reviewed changes

to24toro mentioned this pull request Nov 17, 2023

[WIP] Update rotating frame to use arraylias #289

Closed

add validating type of output of funcs

6567b8f

DanPuzzuoli approved these changes Nov 17, 2023

View reviewed changes

DanPuzzuoli merged commit f44c721 into qiskit-community:main Nov 17, 2023
16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add sparse modules to update dynamics by arraylias #286

Add sparse modules to update dynamics by arraylias #286

to24toro commented Nov 7, 2023

DanPuzzuoli commented Nov 7, 2023

DanPuzzuoli left a comment •

edited

Loading

DanPuzzuoli Nov 7, 2023

to24toro Nov 8, 2023

to24toro Nov 8, 2023

DanPuzzuoli Nov 8, 2023

to24toro commented Nov 8, 2023 •

edited

Loading

to24toro commented Nov 8, 2023 •

edited

Loading

to24toro commented Nov 8, 2023

DanPuzzuoli left a comment •

edited

Loading

DanPuzzuoli Nov 8, 2023

DanPuzzuoli left a comment

DanPuzzuoli left a comment

DanPuzzuoli Nov 16, 2023

DanPuzzuoli Nov 16, 2023

DanPuzzuoli Nov 16, 2023

DanPuzzuoli Nov 16, 2023

		from qiskit_dynamics.type_utils import isinstance_qutip_qobj


		def register_to_numeric_matrix_type(alias):

		from ...common import QiskitDynamicsTestCase


		class TestMultiplyFunction(QiskitDynamicsTestCase):

Add sparse modules to update dynamics by arraylias #286

Add sparse modules to update dynamics by arraylias #286

Conversation

to24toro commented Nov 7, 2023

Summary

Details and comments

DanPuzzuoli commented Nov 7, 2023

DanPuzzuoli left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

to24toro commented Nov 8, 2023 • edited Loading

to24toro commented Nov 8, 2023 • edited Loading

to24toro commented Nov 8, 2023

DanPuzzuoli left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanPuzzuoli left a comment

Choose a reason for hiding this comment

DanPuzzuoli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanPuzzuoli left a comment •

edited

Loading

to24toro commented Nov 8, 2023 •

edited

Loading

to24toro commented Nov 8, 2023 •

edited

Loading

DanPuzzuoli left a comment •

edited

Loading