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

Upgrade pulser-simulation to qutip 5 #783

Merged
merged 13 commits into from
Dec 18, 2024
Merged

Upgrade pulser-simulation to qutip 5 #783

merged 13 commits into from
Dec 18, 2024

Conversation

a-corni
Copy link
Collaborator

@a-corni a-corni commented Dec 16, 2024

Bumps the requirements to qutip 5.
Handles the introduction of the new data type "Dense" in qutip (previously, there was only "CSR"): by default, all the operators used to be initialized as "CSR", but they were now stored as "Dense". This PR converts any Qobj to CSR, such that the performances are kept the same (notably, having CSR in collapse operators is very important, see qutip/qutip#2328).

Closes #730

@a-corni a-corni requested a review from HGSilveri December 16, 2024 17:17
@a-corni a-corni changed the title Hs/qutip 5 Move pulser-simulation to qutip 5 Dec 17, 2024
@HGSilveri HGSilveri added this to the v1.2 milestone Dec 17, 2024
Copy link
Collaborator

@HGSilveri HGSilveri left a comment

Choose a reason for hiding this comment

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

I have a couple of question but it LGTM, very nice job!

pulser-simulation/pulser_simulation/hamiltonian.py Outdated Show resolved Hide resolved
@a-corni
Copy link
Collaborator Author

a-corni commented Dec 17, 2024

For the record, I have investigated the influence of the data type of the initial state. If the initial state is specified with a string (the default behaviour, as in the tutorials above), then the initial state in CSR. If the user defines his own initial state with an array, then the data type of "Dense".

In qutip/qutip#2328, it is said that in the idea of qutip developers, kets should be stored in the data type "Dense" and not "CSR", because CSR @ Dense is faster than CSR @ CSR. I have investigated whether or not having a "Dense" initial state influences the duration of the computation: this is not clear. I drop here the tests I have made by converting all the data to "Dense" before simulation:
benchmark_qutip_5_operators_as_CSR_state_as_Dense.pdf

@a-corni
Copy link
Collaborator Author

a-corni commented Dec 17, 2024

In the end it seems that it is always better to have the initial state in CSR (but it's being picky for few tens of ms) benchmark_qutip_5_operators_as_CSR_state_as_CSR.pdf

@awennersteen
Copy link
Member

Based on general sparse linear algebra CSR x CSR is faster than CSR x CSR if the ket is sufficiently sparse.
IMHO it would be nice to have that user configurable

It's also the sort of thing that can depend a lot on your system

@a-corni a-corni changed the title Move pulser-simulation to qutip 5 Upgrade pulser-simulation to qutip 5 Dec 17, 2024
Copy link
Member

@awennersteen awennersteen left a comment

Choose a reason for hiding this comment

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

lgtm

@a-corni a-corni merged commit 21c8d46 into develop Dec 18, 2024
9 checks passed
@a-corni a-corni deleted the hs/qutip-5 branch December 18, 2024 16:52
@HGSilveri HGSilveri mentioned this pull request Dec 20, 2024
HGSilveri added a commit that referenced this pull request Dec 20, 2024
**Main changes:**
- Fixes to the amplitude noise implementation (#768)
- Add method to Sequence to estimate delay introduced before adding pulse (#773)
- Deprecate usage of int in QubitIds (#774)
- Update displayed device specs (#763) 
- Upgrade pulser-simulation to qutip 5 (#783)
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.

Upgrade to Qutip 5
3 participants