Repository navigation
Conversation
Qrisp's to_cirq has no mapping for u3 and cannot decompose it, so the qrisp -> cirq conversion raised on any u3. qiskit_to_qrisp lowers sdg, tdg, p and u1 to u3, so these failed too; transpile() only succeeded by falling back to qrisp -> qasm2 -> cirq. Rewrite each u3 as rz-ry-rz plus its global phase first.
Qrisp's Cirq converter has no mapping for cy and cannot decompose it, so qrisp_to_cirq raised ValueError on any circuit containing cy, which qiskit_to_qrisp keeps in its basis. Rewrite cy as s_dg, cx, s on the target before calling to_cirq(), conjugating the control with X when the cy has an open control.
Argus reviewAuto-review is off for this repo. Tick the box below to run a review on this PR. Running Argus review... Estimated cost
Tip: you can also comment |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
Qrisp's Cirq converter inlines composite gates after the cy and u3 rewrites have run, so a cy or u3 inside a to_gate() block still raised. Inline the composites that contain one first.
This comment has been minimized.
This comment has been minimized.
![argus-eye[bot]](https://avatars.githubusercontent.com/in/2969822?s=60&v=4){kind=small}
There was a problem hiding this comment.
Argus · 10/10 — No findings
Argus reviewed 3 files and checked 2 for bugs, security, architecture, and regressions. Custom agent ponytail-review also ran. The diff matches the stated goal.
What this PR does · 6 author checks
- Fix qrisp_to_cirq failures on cy and u3 by rewriting them before conversion, including when nested in composite gates.
- Excludes: Fixing controlled composites containing cy that become ccy; qrisp's upstream transpile() decomposition bug remains unchanged.
Author's checks
- Before to_cirq(), rewrite u3(θ, φ, λ) as rz(λ), ry(θ), rz(φ), preserving its global phase.
- Before to_cirq(), rewrite cy as s_dg, cx, s on the target.
- For cy with ctrl_state="0", conjugate the control with X.
- Inline composite gates containing cy or u3 before rewriting them.
- Leave other composite gates for Qrisp's converter, as before.
- Tests compare conversion results against textbook matrices.
usage: 274.9k tokens · 15 stages
Contract: production/full · checked: bug_hunter, security, architecture, regression, single-pass review, ponytail-review · review took 1m21s
| Stage | Model | Tokens | Cost |
|---|---|---|---|
| Intent | openai/gpt-6.1-sol |
3.8k | $0.0079 |
| Triage | openai/gpt-6.1-sol |
6.7k | $0.0078 |
| Lead agent | openai/gpt-6.1-sol |
1.3k | $0.0066 |
| Review specialists (10) | openai/gpt-6.1-sol |
232.7k | $0.4872 |
| Review | openai/gpt-6.1-sol |
27.8k | $0.0559 |
| Simulation | openai/gpt-6.1-sol |
2.7k | $0.0081 |
Dashboard → · @argus-eye help
🧠 Learned: 1 pattern · 1 PR summary · 1 architecture note
qrisp_to_cirqraised on any circuit withcyoru3. Qrisp's Cirq converter has no mapping for either and can't decompose them.qiskit_to_qrispkeepscyin its basis and lowerssdg,tdg,pandu1tou3, so ordinary Qiskit circuits failed on this edge.Both are now rewritten before
to_cirq():u3(θ, φ, λ)becomesrz(λ) ry(θ) rz(φ)plus its global phase.cybecomess_dg,cx,son the target, with the control conjugated byXwhen it is open (ctrl_state="0").Composite gates (
to_gate()) that contain acyoru3are inlined first. Qrisp's converter would otherwise inline them itself after the rewrite, and the nested gate would still raise. Other composites are left for the converter, as before.Not covered: a composite under a control (
to_gate().control(1)) that containscybecomes accy, which qrisp's owntranspile()decomposes wrongly. That's an upstream qrisp bug and is unchanged frommain.Found by a conversion conformance fuzzer that checks each conversion against an independently computed unitary. The tests compare against textbook matrices.