-
Notifications
You must be signed in to change notification settings - Fork 31
Use FillArrays.jl for handling superoperators
#589
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors superoperator handling to use FillArrays.Eye instead of standard identity matrices I(N), improving memory efficiency and code cleanliness. The lightweight Eye constructor eliminates unnecessary vector allocations while maintaining the same interface.
- Replaced all
I(N)identity matrix constructions withEye(N)in superoperator functions - Removed
Id_cacheparameters from public APIs (spre,spost,lindblad_dissipator,liouvillian) - Added FillArrays.jl as a dependency (already used by other SciML packages)
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Project.toml | Added FillArrays dependency with version constraint "1" |
| src/QuantumToolbox.jl | Imported Eye from FillArrays |
| src/qobj/superoperators.jl | Replaced I() with Eye() throughout; removed Id_cache parameters from all superoperator functions |
| src/spin_lattice.jl | Updated multisite_operator to use Eye() in kronecker products |
| src/time_evolution/smesolve.jl | Removed Id parameter from _spre and _spost calls |
| src/time_evolution/brmesolve.jl | Removed Id parameter from _spre and _spost calls |
| src/time_evolution/time_evolution_dynamical.jl | Removed dsf_identity from internal parameters passed between functions |
| CHANGELOG.md | Documented the change with reference to issue #589 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ytdHuang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest we add deprecated warnings for spre, spost, liouvillian, and lindblad_dissipator, and ignore the argument Id_cache, namely
function spre(A::AbstractQuantumObject{Operator}, Id_cache)
Base.depwarn(
"The argument `Id_cache` for `spre` is now deprecated, as we now internally use FillArrays.jl, which preserves the same efficiency without requiring this parameter.",
:spre,
force = true,
)
return spre(A)
end|
Since our HEOM package relies a lot on Basically, just do the similar changes in this PR for HEOM. |
|
The documentation build is failing due to the doctest for But since I will change it again in the next PR, we can keep it as it is at the moment. |
e3ad9ce to
1b5718a
Compare
Checklist
Thank you for contributing to
QuantumToolbox.jl! Please make sure you have finished the following tasks before opening the PR.make test.juliaformatted by running:make format.docs/folder) related to code changes were updated and able to build locally by running:make docs.CHANGELOG.mdshould be updated (regarding to the code changes) and built by running:make changelog.Request for a review after you have completed all the tasks. If you have not finished them all, you can also open a Draft Pull Request to let the others know this on-going work.
Description
The superoperators are obtained by making the Kronecker product to the given operator. This requires to have the identity matrix.
The previous implementation was using the standard identity
I(N), allocating the vector of the diagonal. This is inefficient, as the vector allocates but we already know it is filled by only ones by definition. So, we previously used theId_cacheargument, to avoid to repeatedly allocate.Here I use instead the
Eyeconstructor from FillArrays.jl. It is a lightweight constructor to replicate the identity matrix. Is has the same size of a numberThis makes the code way cleaner. Avoiding to define the cache.
The use of FillArrays doens't increase the number of dependencies, as it is already a dependency of many SciML packages.