Repository navigation
Conversation
098d5bf to
380dafb
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3275 +/- ##
=======================================
Coverage 96.25% 96.25%
=======================================
Files 177 177
Lines 21699 21734 +35
Branches 2162 2168 +6
=======================================
+ Hits 20886 20921 +35
Misses 629 629
Partials 184 184 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Now that there's more decomposition transforms, things started checking the wrong qnode. So just need to anchor each qnode.
There was a problem hiding this comment.
Just using skip_preprocess on all the drawing tests. XDSL can be a follow up feature if needed. But drawing can't handle all the new classical processing introduced, like SliceOp.
There was a problem hiding this comment.
Something about functions with both quantum and classical instructions causes differentiation to fail.
There was a problem hiding this comment.
So gridsynth currently gets run on all the decomposition rules in addition to the qnode body. This makes it so mixed-fallback introduces mcm's inside adjoint regions, which then causes errors in the decomposition pass.
| pipe = [("pipe", ["quantum-compilation-stage"])] | ||
|
|
||
| @qjit(pipelines=pipe, target="mlir", capture=True) | ||
| @qjit(pipelines=pipe, target="mlir", capture=True, skip_preprocess=True) |
There was a problem hiding this comment.
So none of the PPR ops are actually supported by null qubit 🤷
There was a problem hiding this comment.
https://xanaduhq.slack.com/archives/C030UB33QH3/p1790965280121669
I think this is relevant here, although I know @albi3ro had concerns on whether this was the most appropriate convention.
|
|
||
|
|
||
| def test_controlled_pauli_rot_failure(): | ||
| def test_controlled_pauli_rot_works(): |
There was a problem hiding this comment.
🎉 my favourite type of test update.
mudit2812
left a comment
There was a problem hiding this comment.
Looks great to me! Just one comment regarding the gridsynth tests
3f63e4a to
8b7e753
Compare
|
|
||
| pipeline = _create_mlir_cli_schedule(pass_ops) | ||
| modified = _quantum_opt(*pipeline, "-mlir-print-op-generic", stdin=buffer.getvalue()) | ||
| modified = _quantum_opt( |
There was a problem hiding this comment.
Compared to the previous (no-capture) pipeline, decomposing circuits after measurement transforms is right, but is decomposing after validation and after dynamic-one- shot okay? Validation is a no-op under capture rn, so that part doesn't break anything for now. But any rule that adds mcms would bypass the one-shot handling.
| def test_paramshift_with_gates(gate, state, capture_mode): | ||
| """Test parameter shift works with a variety of gates present in the circuit.""" | ||
|
|
||
| if gate is qp.BasisState and capture_mode: |
There was a problem hiding this comment.
Any plans to fix this? BasisState doesn't need to be decomposed as Lightning devices can support these ops re to capabilities.initial_state_prep. We also only test it with param-shift rn, could you test the device adjoint?
There was a problem hiding this comment.
So the problem is that BasisState had different interpretation in pennylane and catalyst. One was more of a "xor bit flip" interpretation, and one was a more "prepare this state" interpretation. Since existing templates where using it in the "xor bit flip" interpretation, we couldn't lower it to a state prep instruction.
As for fixing gradients, something about functions with both quantum and classical stuff in it is causing issues. I looked into fixing it via an inlining pass, but we can only inline functions with quantum instructions if the function has a target gate attribute. As far as I can tell, no one knows why and how this restriction got added.
There was a problem hiding this comment.
BasisState had different interpretation in pennylane and catalyst
I don't think this is a difference in PL vs Catalyst, Catalyst simply reimplemented the distinction already in PL with regards to the "initial state prep" in a program. Catalyst relied on the decomposition in the frontend to select when to decompose and when not, and any remaining state prep were assumed to be of the "initial" type.
There was a problem hiding this comment.
With the decomposition moved to the compiler we'd have to replicate this decision somehow, but the graph isn't contextual at the moment. But in principal the quantum.operator version of BasisState could be the generic one (i.e. unitary semantics) while the dedicated ops could be reserved for the initial kind (i.e. set state semantics).
|
|
||
|
|
||
| # pylint: disable=unused-argument | ||
| def _gateset_preprocessing( |
There was a problem hiding this comment.
the gateset literally ignores the gradient method / compatible gates. In the current no-capture pipeline), when a gradient is requested, the old frontend catalyst_acceptance also decomposes ops the device can't differentiate. On lightning those are Rot, QubitUnitary, PauliRot, etc.
There was a problem hiding this comment.
Yep. This also literally breaks gradient support as a whole. So we are going to have to come back to gradients again whenever product decides it is a priority.
| """ | ||
|
|
||
| @qjit(capture=True, collect_decomp_rules=False) | ||
| @qjit(capture=True, collect_decomp_rules=False, skip_preprocess=True) |
There was a problem hiding this comment.
Because otherwise it will try to decompose BasisState, and we can't handle a decomposition rule on more than one quantum register. hence why have to go back to the drawing board on how allocation works.
| object code or IR. The default is stdout for IR and `<module-name>.o` for object code. | ||
| Stdout (i.e. `-o -`) is rejected when the output is object code. | ||
|
|
||
| * Decomposition to the device's gateset is now applied by default with `capture=True`. |
There was a problem hiding this comment.
| * Decomposition to the device's gateset is now applied by default with `capture=True`. | |
| * Decomposition to the device's gate set is now applied by default with `capture=True`. |
| This may introduce performance slowdowns or failures with cases that are not yet supported by the | ||
| decomposition pass. In the event of issues, this can be turned off with `skip_preprocess=True` being | ||
| passed to `qjit`. | ||
| [(#3275)](https://github.com/PennyLaneAI/catalyst/pull/3275) |
There was a problem hiding this comment.
should we add it to "Breaking Changes" as it breaks the default behaviour? 🤔
There was a problem hiding this comment.
added to breaking changes.
880a02e to
8790fae
Compare
…has no following block
…support with device gateset
…ified Compiler (#3276) ### Before submitting Please complete the following checklist when submitting a PR: - [ ] All new functions and code must be clearly commented and documented. - [ ] Ensure that code is properly formatted by running `make format`. The latest version of black and `clang-format-20` are used in CI/CD to check formatting. - [ ] All new features must include a unit test. Integration and frontend tests should be added to [`frontend/test`](../frontend/test/), Quantum dialect and MLIR tests should be added to [`mlir/test`](../mlir/test/), and Runtime tests should be added to [`runtime/tests`](../runtime/tests/). When all the above are checked, delete everything above the dashed line and fill in the pull request template. ------------------------------------------------------------------------------------------------------------ **Context:** **Description of the Change:** **Benefits:** **Possible Drawbacks:** **Related GitHub Issues:** Used Cursor --------- Co-authored-by: Nikhil Sreekumar <nikhil.sreekumar@xanadu.ai> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Christina Lee <chrissie.c.l@gmail.com>
8790fae to
578bc84
Compare
Context:
Add a
graph-decompositionpass based on the devicetomlbackend.Description of the Change:
Automatically run the
graph-decompositionpass based on the device's target gateset.frontendfor ensuring pass is added successfully through device preprocessing.xDSLpath does not parsegraph-decompositioncorrectly when offloadingnon-xDSLpasses toCatalystwithin thetransform.sequenceBenefits:
graph-decomposition.Possible Drawbacks:
pytestsneed to be updated to reflect when device preprocessing is necessary for the test.Related GitHub Issues:
[sc-129198]