Skip to content

Decomp/xas misc fixes - #3183

Draft
paul0403 wants to merge 32 commits into
mainfrom
decomp/xas-misc-fixes
Draft

paul0403 wants to merge 32 commits into
mainfrom
decomp/xas-misc-fixes

Conversation

@paul0403

@paul0403 paul0403 commented Sep 3, 2026

Copy link
Copy Markdown
Member

Context:
Miscellaneous fixes found out while testing XAS.

@kipawaa
kipawaa force-pushed the decomp/xas-misc-fixes branch from 94c447a to 356abee Compare September 9, 2026 15:56
Comment on lines 826 to 830
inlined = _quantum_opt(
"--inline=inlining-threshold=4294967295", # Use uint max to indicate always inline
"--mlir-print-op-generic",
stdin=str(module),
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that the always-inline pass doesn't reach func.calls nested inside qref.ctrl / adjoint regions, so pjit helpers survive there. whether those ops implement RegionBranchOpInterface/the inliner interface, or whether inlining should run before ctrl/adjoint wrapping?
In one of my workflow compilation I saw _right_givens_jax survives — and every failing loc is ctrl/… or adjoint/… prefixed (ctrl/jit(angle), adjoint/jit(remainder), ctrl/adjoint/for/body/jit(allclose)).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/PennyLaneAI/catalyst/blob/main/mlir/test/QRef/InlineTest.mlir are you sure? There are tests for calls in adjoint and control regions

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/PennyLaneAI/catalyst/blob/main/mlir/test/QRef/InlineTest.mlir are you sure? There are tests for calls in adjoint and control regions

I'm not so sure now since my local branch also changed a lot since last time I reported this 😆

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me take a second look

@maliasadi
maliasadi marked this pull request as draft September 11, 2026 20:04
"""Return a dictionary of wire arg names to lengths."""
wire_lens = {}
for wire_name, wire_arg in sorted(self.op.wire_args.items()):
if wire_name not in self.op.hybrid_argnames:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if wire_name not in self.op.hybrid_argnames and len(wire_arg) > 0:

When compiling EELS, I saw SumOfSlatersPrep with an identification_wires register that legitimately has zero wires — required_register_sizes returns 0 for these index sets. Python's parse_wire_lens still writes it into the ID as identification_wires:0, but the compiler builds its ID from qubit operands, and an empty register contributes none, so it leaves the entry out entirely. The two IDs never match, the sos_state_prep rule ends up registered against a node no operator instance maps to, and the solver reports "Decomposition rule not found for SumOfSlatersPrep"
I tried skipping empty register here and EELS works. But not sure if this is a proper fix without bad side effects @maliasadi

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean the MLIR op cannot have a qubit_index name that is indexing nothing 😅

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants