feat: SciMLOperators-backed lazy operator extension and benchmark suite#233
feat: SciMLOperators-backed lazy operator extension and benchmark suite#233Adithyaphani wants to merge 3 commits into
Conversation
|
@Krastanov looking forward to your response and review on this PR , happy to make further changes if needed. |
|
@Krastanov Pushed a fix — the first CI run flagged one issue: the |
|
@Krastanov Build #114 is green for the SciMLOperators extension — test/test_sciml_lazy_operators.jl now passes 45/45 with no errors. The 2 remaining failures (test_plotting.jl — Makie not found, and test_sciml_broadcast_interfaces.jl — DP5 undefined) are in files this PR doesn't touch. The DP5 one looks like a namespace change in a recent OrdinaryDiffEqLowOrderRK release; the Makie one looks like an environment resolution issue. Both appear unrelated to this change — happy to look into either if useful, but didn't want to bundle unrelated fixes into this PR. Looking forward to your review and response. |
|
@Krastanov looking forward to your review & response on this PR, and happy to make further changes if needed. |
|
@Krastanov All CI platforms are green for our changes — test/test_sciml_lazy_operators.jl passes 45/45 on Linux, macOS, and Windows. CI / Julia 1 (macOS, ubuntu, windows) + Julia 1.10 — same 2 errors on all 4 platforms: DP5 moved to OrdinaryDiffEqLowOrderRK namespace (test_sciml_broadcast_interfaces.jl), and Makie not found on test path (test_plotting.jl). Neither file is touched by this PR. Happy to dig into any of these if it'd help, but didn't want to bundle unrelated fixes into this PR. @Krastanov looking forward to your response. |
|
@Krastanov looking forward to your response for the above comment. |
@Krastanov looking forward to your reply on this comment and happy to make further changes if needed. |
|
@Krastanov looking forward to your review on this pr. |
|
@Krastanov looking forward to your review on the pr and happy to make further changes if needed. |
1 participant
Closes qojulia/QuantumOptics.jl#522
Summary:
Adds an optional package extension that converts QuantumOpticsBase's lazy operators (LazySum, LazyProduct, LazyTensor) into a SciMLOperators-backed representation, along with correctness tests and a benchmark suite comparing both approaches. SciMLOperators is wired in as a weak dependency, so nothing changes for users who don't load it.
What's included
ext/QuantumOpticsBaseSciMLOperatorsExt.jl — the extension, providing sciml_lazy_operator(op) and cache_sciml_lazy_operator(w, u)
test/test_sciml_lazy_operators.jl — 23 @testitem blocks, 32 assertions, all passing
benchmark/sciml_lazy_benchmark.jl — BenchmarkTools comparison across LazySum, LazyProduct, LazyTensor (dense/sparse, various sites and local dimensions), and a mixed Heisenberg-like Hamiltonian
Project.toml updates for the new weak dependency/extension
Two correctness issues found and fixed during implementation:
Index ordering mismatch — QuantumOpticsBase uses column-major ordering (site 1 fastest-varying), while SciMLOperators' TensorProductOperator uses row-major (first argument slowest-varying). These are opposite conventions. Without reversing the site operator order, LazyTensor conversions construct without error but apply to the wrong tensor factor — a silent correctness bug. Fixed with a single reverse(), documented in the source.
Cache requirement — Any operator tree containing a TensorProductOperator requires cache_operator(L, u) before mul!, unlike MatrixOperator. dense() and out-of-place * auto-cache internally (fine for one-off use); cache_sciml_lazy_operator pre-allocates for repeated application in hot loops.
Tests:
All 23 test items pass (32 assertions). Pre-existing tests unaffected.
Benchmark:
Script is checked in and runnable via julia --project=benchmark benchmark/sciml_lazy_benchmark.jl, covering the cases listed in the issue's benchmark requirements. I'll post numbers from a clean local run as a follow-up comment.
Out of scope:
No changes to the default LazySum/LazyProduct/LazyTensor paths, no GPU/time-dependent operator support. dagger currently materializes the adjoint as a one-time cost — a known prototype limitation.
Open question:
The native LazyTensor path has lazytensor_cache enabled by default. The benchmark compares both paths with default settings — happy to add a cache-disabled column if more useful.
AI use disclosure:
Used Claude to understand the SciMLOperators API (TensorProductOperator semantics, cache_operator behavior). Both correctness findings above were discovered by running tests and reading failure output, not by the model. All code written, tested, and verified locally.