qiskit-community / qiskit-dynamics

Tools for building and solving models of quantum systems in Qiskit
https://qiskit-community.github.io/qiskit-dynamics/
Apache License 2.0
105 stars 61 forks source link

Update rotating_frame to use arraylias #290

Closed to24toro closed 12 months ago

to24toro commented 1 year ago

Summary

I updated rotaing_frame.py. The content of this PR is the same as https://github.com/Qiskit-Extensions/qiskit-dynamics/pull/289 but the direction of the merged PR is changed to models_arraylias_integration not main.

Details and comments

I've confirmed the test about the rotating_frame has passed successfully.

to24toro commented 1 year ago

My main comments are on the tests. For the sparse types, I was thinking it makes sense to add a JAXBCOOTestBase and a SciPySparseTestBase with array_library "jax_sparse" and "scipy_sparse". Then, e.g. you can do: TestRotatingFrameJAXBCOO(JAXBCOOTestBase) Then, self.asarray will produce BCOO directly.

Yes. It is what I want to discuss.

In many tests, we compare between the output of self.asarray() and that of BCOO.todense(). It's necessary to create two arrays: self.asarray() and BCOO.todense(). Writing codes for these arrays is not reduced. The usage of self.asaray() is designed for multiple classes(e.g. JAX and numpy). However, looking at BCOO tests in some folders, they seem to be specific to BCOO. Is it necessary to use self.asarray() in these classes?

On the other hand, you mentioned the integration of OperationCollection classes in another PR. If we integrate them, it might be beneficial to add sparseTestBase classes. I'm not very familiar with how the configuration of tests in other folders looks, but I can add these new sparsetestbase classes now if it seems likely that we will need them in the future.