Skip to content

fix: reject quantum.adjoint input in ppr-to-ppm with a clear error#3020

Open
mvanhorn wants to merge 2 commits into
PennyLaneAI:mainfrom
mvanhorn:fix/2934-reject-adjoint-ppr-to-ppm
Open

fix: reject quantum.adjoint input in ppr-to-ppm with a clear error#3020
mvanhorn wants to merge 2 commits into
PennyLaneAI:mainfrom
mvanhorn:fix/2934-reject-adjoint-ppr-to-ppm

Conversation

@mvanhorn

@mvanhorn mvanhorn commented Jul 14, 2026

Copy link
Copy Markdown

Before submitting

  • All new functions and code must be clearly commented and documented.

  • Ensure that code is properly formatted by running make format.

  • All new features must include a unit test.


Context:

As reported in #2934, applying the to_ppr and then ppr-to-ppm transforms to a circuit that
contains a functional adjoint (e.g. qp.adjoint(lambda: qp.S(0))()) crashes the compiler with a
raw MLIR assertion:

Assertion failed: (result && "expected 'from' to be contained within the map"),
function lookup, file IRMapping.h, line 74

surfaced to the user as CompileError: catalyst failed with error code -6.

The root cause is a pass-ordering interaction. The default quantum-compilation-pipeline runs
adjoint-lowering automatically after the user transforms, and that pass reverses a quantum.adjoint
region by walking its ops. adjoint-lowering can reverse pbc.ppr ops (see
mlir/test/PBC/AdjointTest.mlir), but once ppr-to-ppm has turned the region's PPRs into Pauli
product measurement (pbc.ppm) ops, those measurements are not reversible, so the reversal hits the
IRMapping assertion instead of reporting anything actionable.

Description of the Change:

Following the maintainer's direction on the issue to tighten the pass's input requirements at the
start of the PBC conversion rather than failing later, this adds an input-validation guard at the
very start of PPRToPPMPass::runOnOperation, before any patterns are applied. The pass now walks
its operation for a quantum.adjoint op and, if one is found, emits a clear error explaining that
ppr-to-ppm cannot be applied inside a quantum.adjoint region because the resulting Pauli product
measurements cannot be adjoint-lowered, then signals pass failure.

The guard is scoped to ppr-to-ppm only; to_ppr produces PPRs, which adjoint-lowering handles
correctly, and is left unchanged.

A lit test is added to mlir/test/PBC/PPRToPPM.mlir that wraps a pbc.ppr in a quantum.adjoint
region and asserts, via -verify-diagnostics, that the pass emits the new diagnostic and fails
cleanly instead of asserting.

Benefits:

A malformed input that previously aborted the compiler with an internal assertion (error code -6)
now produces a clear, actionable diagnostic that names the unsupported construct and the location of
the offending op.

Possible Drawbacks:

The guard is deliberately conservative: any quantum.adjoint op reaching ppr-to-ppm is rejected,
matching the maintainer's "no unexpected ops through the pass" framing, rather than attempting to
detect whether a given adjoint region would in fact receive generated measurements.

Related GitHub Issues:

Fixes #2934


AI tool disclosure (per the PennyLane AI Tool Use Policy): this contribution was developed with AI assistance (Claude Code, and Codex for follow-up revisions). I reviewed all generated code and text, wrote this description myself, and am accountable for the work as its author. Happy to answer questions about the implementation during review.

The ppr-to-ppm pass converts PPR ops into Pauli product measurement
(pbc.ppm) ops, which the adjoint-lowering pass cannot reverse. When the
input contained a quantum.adjoint region, the later adjoint-lowering
walk hit a raw IRMapping assertion instead of a clear error.

Guard PPRToPPMPass::runOnOperation to walk for a quantum.adjoint op and
emit a clear diagnostic before any PPMs are generated, then fail the
pass. Adds an MLIR lit test asserting the diagnostic and a changelog
entry.

Fixes PennyLaneAI#2934

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QXwvFKU1BuKiw1hKwr8fwC
@github-actions github-actions Bot added the external PRs where the author is not a part of PennyLane Org (or part of external contributors team) label Jul 14, 2026
@mehrdad2m

Copy link
Copy Markdown
Contributor

Hi @mvanhorn, welcome to Catalyst, and thanks for the PR! 🎉

The team will review shortly. The changes looks good overall, but just one thing to flag for discussion. While I think the input rejection at the ppr-to-ppm is nice to have, I am wondering if we could also raise a better error at the adjoint lowering pass as well since this pattern could happen with other user passes that would introduce incompatible ops for adjoint lowering. tagging @dime10 as well since he suggested the direction.

Also note that to be compliant with our policy you must label the AI agent used to assist you in the PR description.

Generalize the reverse-pass guard so any operation left unhandled that
produces or consumes quantum values (including non-Quantum-dialect ops
such as pbc.ppm) emits a clear diagnostic and fails the pass, instead of
falling through to a raw IRMapping assertion. Purely classical ops are
still skipped. Adds a lit test and a changelog entry for PennyLaneAI#2934.
@mvanhorn

Copy link
Copy Markdown
Author

Good call, and agreed the adjoint-lowering pass is the right place for the general fix. Pushed in 1010942: the reverse-pass dispatch now emits a clear error for any op it leaves unhandled that produces or consumes quantum values, including non-Quantum-dialect ops like pbc.ppm, instead of falling through to the raw IRMapping assertion. Purely classical ops are still skipped, so it only fires on ops that actually participate in the quantum data flow. That covers the case @dime10 raised where another user pass introduces incompatible ops, not just ppr-to-ppm. I kept the ppr-to-ppm guard too since it gives an earlier, more specific message. Added a lit test with a pbc.ppm inside a quantum.adjoint region asserting the diagnostic, plus a changelog entry. clang-format is clean; I could not run the MLIR lit/build locally (no LLVM build in my environment), so I am relying on CI for that. Also added the AI-tool disclosure to the PR description per your policy.

@dime10

dime10 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Thank you for working on this issue! I think where we landed on is that the PBC conversion needs to be better built out as a change in representation: from the generic gate model (quantum dialect) to the pauli-based representation (pbc dialect). Right it now, it is a basic pass that targets PPR gates from certain quantum ops.

Adding a narrow check for adjoint op is helpful, but doesn't exactly fix the broader issue. Roughly, I had the following in mind:

  • reframe the pass (name, docs, etc) as a full semantic conversion from the quantum to the pbc dialect
    • it doesn't necessarily have to use the dialect conversion infrastructure, although it can (it's know to be slower than "manual" passes, but we still use it because it is easy to work with)
  • use an allowlist for operations we know how to convert (e.g. quantum.custom), or can safely remain in the IR (e.g. quantum.alloc)
    • this is counter to a blocklist approach where we explicitly mark certain ops as not allowed (e.g. quantum.adjoint)
    • the reason the allowlist is better is because a blocklist needs to be kept up to date regularly (whenever the quantum dialect has new operations), even when those ops are unrelated to PBC (which means the author will not think of the PBC conversion blocklist)
  • this part may require some discussion, but we could invoke any necessary steps required as part of the conversion automatically, for example the adjoint lowering pass
    • can be done as a nested pass invocation

@mvanhorn

Copy link
Copy Markdown
Author

That framing makes sense to me, and I'd rather build it the right way than bolt adjoint on as a special case. Reworking it as a full quantum -> pbc semantic conversion (renaming the pass and updating the docs to match) sounds like the right direction.

On allowlist vs blocklist - agreed, and for exactly the reason you gave: a blocklist silently rots every time the quantum dialect gains an op, since nobody adding a new op is thinking about the PBC conversion. I'll switch to an allowlist of ops we know how to convert (quantum.custom) or can safely leave in place (quantum.alloc), and reject anything else by default.

The part I'd like your steer on is the automatic conversion steps:

  1. For adjoint specifically, do you want the pass to invoke adjoint lowering itself as a nested pass, or to assume adjoints are already lowered upstream and just error on a quantum.adjoint it sees? A nested invocation is nicer for callers but couples this pass to the adjoint-lowering pipeline.
  2. Build it on the dialect conversion framework, or a manual pass? You mentioned the framework is slower but easier to work with - I'd lean manual for a conversion this targeted, but happy to use the framework if you'd rather keep it consistent with the rest of the codebase.

Once I know your preference on those two I'll rework the pass accordingly.

@dime10

dime10 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

@mehrdad2m Any thoughts here?

@mehrdad2m

Copy link
Copy Markdown
Contributor

@mehrdad2m Any thoughts here?
Thanks both,
I agree with the overall framework discussed here. on the open questions:

  • On adjoint, I think the nested approach is nice to have. The only downside is that nesting passes in this pipeline has the potential to confuse users who are modifying the pipeline, but as long it is documented, I think it should be fine.

  • on adjoint, I think the nested approach is nice to have as it makes the transforms self-contained. The only downside is that nesting passes in this pipeline has the potential to confuse users who are modifying the pipeline, but as long as it is well documented, I think it should be fine.

  • On the pass framework, I don't have a strong preference here. However, I slightly lean toward the dialect conversion framework as it will make the maintenance easier and natively support the allowlist discussed. We could also consider potentially using the no rollback mode in future if the performance becomes an issue, though that has its own cons. That being said, I am fine with the manual pass as well as long as the pass implementation is simple enough to maintain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external PRs where the author is not a part of PennyLane Org (or part of external contributors team)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

adjoint-lowering pass is unable to handle non-Quantum dialect ops

3 participants