🔥 Remove density matrix support from DD package#1466
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
6335c90 to
bbae3d0
Compare
Signed-off-by: burgholzer <burgholzer@me.com>
bbae3d0 to
d6a9762
Compare
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR removes density-matrix (dNode/dEdge) support and stochastic/deterministic noise functionality from the DD package, replaces many SFINAE-based templates with C++20 concepts (IsVector/IsMatrix), and updates related APIs, compute-tables, bindings, and tests to reflect a vector/matrix-only implementation. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
participant Caller
end
rect rgba(200,255,200,0.5)
participant Package
end
rect rgba(255,200,200,0.5)
participant ComputeTable
end
rect rgba(255,255,200,0.5)
participant UniqueTable
end
Caller->>Package: trace(mEdge a, numQubits)
Package->>ComputeTable: lookup(leftOperand, rightOperand)
alt cache hit
ComputeTable-->>Package: cached mCachedEdge
Package->>UniqueTable: use cached result / finalize
UniqueTable-->>Package: node/result
Package-->>Caller: ComplexValue result
else cache miss
ComputeTable-->>Package: miss
Package->>UniqueTable: compute trace recursively (trace(..., eliminate, level,...))
UniqueTable-->>Package: computed mCachedEdge
Package->>ComputeTable: insert(computed)
Package-->>Caller: ComplexValue result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
include/mqt-core/dd/DDDefinitions.hpp (1)
59-65: LGTM!Using
std::numbers::piis cleaner and more maintainable than the hardcoded literal. Note thatPI_2andPI_4still use literals—consider updating them tostd::numbers::pi / 2andstd::numbers::pi / 4for consistency if compile-time evaluation is acceptable.src/dd/Edge.cpp (2)
53-67: Guard against out-of-range decision digits.
Line 65 uses<=, which permitstmp == sizeand can lead to out-of-bounds access in release builds if a bad digit slips in.🛠️ Proposed fix
- assert(tmp <= r.p->e.size()); - r = r.p->e[tmp]; + assert(tmp < r.p->e.size()); + if (tmp >= r.p->e.size()) { + return 0.; + } + r = r.p->e[tmp];
439-453: EnforcenumQubits > p->vinprintMatrix.
Line 452 allows equality even thoughnumQubitsis a count, which can mask underspecified calls.🛠️ Proposed fix
- assert(isTerminal() || numQubits >= p->v); + assert(isTerminal() || numQubits > p->v);
Signed-off-by: burgholzer <burgholzer@me.com>
Description
This PR removes the density matrix functionality from the DD package in MQT Core.
It was only ever used in MQT DDSIM and was never really neatly integrated into the DD package; leaving room for improvement.
As a consequence, and in line with the upcoming
v4release, this PR removes the respective functionality and trims down the library.In the process it applies C++20 concepts and ranges wherever appropriate to simplify the code.
Fixes #337
Resolves #826
Checklist: