Feat cirq custom gate by MathieuG-Colibri · Pull Request #200 · ColibrITD-SAS/mpqp

MathieuG-Colibri · 2026-05-20T15:02:46Z

Change the cirq custom gate implem and behavior
Cleanup and bug fixing

Refactored to_other_language

Henri-ColibrITD · 2026-06-19T07:58:43Z

+             ┌──────────┐
+        q_0: ┤2         ├
+             │          │
+        q_1: ┤1 Unitary ├
+             │          │
+        q_2: ┤0         ├
+             └──────────┘


this looks like a regression to me: previous version seemed more representative

Yes but it made that we couldn't translate them back (cannot decompose into qasm).

So it had translatability issues.

there is a difference between display and what's actually stored, see this comment: #200 (comment)

done with print option df88af6

Henri-ColibrITD · 2026-06-19T08:36:10Z

+from typing import Any, Optional
+
+from mpqp.environment.var_cache import (
+    _INSTALLED_MPQP_PROVIDERS,  # pyright: ignore[reportPrivateUsage]


is there a reason to have it private ? I don't remember

I don't know either I just know it was done that way before.

Henri-ColibrITD · 2026-06-19T08:56:33Z

+    if not is_unitary(v):
+        v = closest_unitary(v)


we could move this test in "clostest_unitary"

done df88af6

Henri-ColibrITD · 2026-06-19T10:08:51Z

-            circuit.add(rotation(angle, position))
-        circuit.add(CNOT(control + position, position))
+            circuit.add(rotation(angle, int(targets[position])))
+        circuit.add(CNOT(int(control + targets[position]), int(targets[position])))


why int casting ?

It was a bug with a test where the typing got messed up here and it broken when adding gates.
So it's a failsafe that I put here because I never saw it happen anywhere else.

maybe we should rather fix the test ?

Henri-ColibrITD · 2026-06-19T10:10:11Z

not part of the PR but not super clear

Henri-ColibrITD · 2026-06-19T10:10:28Z

        changed = _gray_code(i) ^ _gray_code(i + 1)
        control = next(i for i in range(len(thetas)) if (changed >> i & 1))
-        control = max(-control - position - 1 + circuit.nb_qubits, 1)
+        control = max(-control - targets[position] - 1 + circuit.nb_qubits, 1)


linked to previous comment

Henri-ColibrITD · 2026-06-19T10:11:31Z

+        if abs >= 1:
+            beta = 0
+        else:
+            beta = 2 * math.acos(abs)


small preference for ternary here but you can ignore this comment if you will

done df88af6

Henri-ColibrITD · 2026-06-19T10:12:01Z

+        circuit.add(Rz(alpha, int(targets[position])))
+        circuit.add(Ry(beta, int(targets[position])))
+        circuit.add(Rz(gamma, int(targets[position])))


same as before, why the int casting ? targets is supposed to be a list of ints

removed all int casting df88af6

The issue seems to be fixed

Henri-ColibrITD · 2026-06-19T10:21:49Z

+    InstalledProviders,
+)
+
+if InstalledProviders.BRAKET in _INSTALLED_MPQP_PROVIDERS:


braket ? cirq ?

fixed df88af6

Henri-ColibrITD · 2026-06-19T12:16:32Z

moving the export to another file, why not, but maybe we should give it some more thought ? because as is we are duplicating some code for each supported language

My main reason for this change is that in some translation context we might need provider specific parameters that would be tougher to keep clean in one to_other_language function.

Henri-ColibrITD · 2026-06-19T12:19:06Z

+
+                # This method adds a lot of identities so for a much cleaner circuit we remove them.
+                # Maybe add this as an option
+                c.instructions = [i for i in c.instructions if not isinstance(i, Id)]


what are we doing with c ? it seems to me like we completely ignore it once built

refactored df88af6

Henri-ColibrITD · 2026-06-19T12:20:53Z

+        # If the CustomControlledGate contains itself a custom gate it's better to return a bigger custom gate for compatibilities issues.
+        # If the non_controlled_gate is a NativeGate it shouldn't pose a problem.
+        # TODO: check how to decompose a ComposeGate that is inside a CCG, (example: a C-PRX)


maybe have an exception for circuit drawing ? to solve the first comment I wrote in the PR

done df88af6

Henri-ColibrITD · 2026-06-19T12:22:18Z

+    return [gate]
+
+
+def mpqp_to_qiskit(


a inconvenient of moving the code here is that it makes the job of reviewing much trickier

could we have a meeting to recap what changed ? it would save me some time ^^

Maybe this was already discussed but I would like to know the reason why we would keep this here? (I take liberty to unresolve the comment)
I meen mpqp.tools.circuit is supposed to be for tools on manipulation of circuit. Anything related to translation should have another dedicated file/location in the code I believe

@MathieuG-Colibri so my suggestion, as discussed this week, is to regroup all translation/conversion features and code in a new module/package mpqp.translationor whatever.
The idea is to regroup all specifc functions/block of code called in to_other_language and from_other_language, or other helper functions or tools related to translation. This should also include the package concerning qasm conversion, (we would typically now have mpqp.translation.qasm instead of mpqp.qasm for instance)

Co-authored-by: Henri de Boutray <133855265+Henri-ColibrITD@users.noreply.github.com>

MoHermes · 2026-07-01T09:46:48Z

+
+    Note:
+        If the circuit contains ComposedGate type instructions you need to include the gate in the authorize_gates set for it to avoid decomposition, otherwise, the translation will always
+        try to use the gate's decomposition (see example). Also in the case of CustomGates it will always be decomposed into the {Ry, Rz, CNOT} basis for translation compatibilities.


To update docs, we no longer decomposed into the {Ry, Rz, CNOT}

Solved (forgot which commit)

MoHermes · 2026-07-01T09:54:44Z

+    for instruction in circuit.instructions:
+        if isinstance(instruction, (Measure, Breakpoint)):
+            continue
+        options = {"printing": printing} if isinstance(instruction, CustomGate) else {}


So, printing=True apply only when the instruction is CustomGate?, I mean it is also propagated CustomControlledGate(...)?

Yes printing was used at the beginning to make sure we could print parametrized CG, I did something similar with Custom Controlled Gates.

MoHermes · 2026-07-01T10:07:41Z

            if isinstance(gate, Operator):
                gate = gate.to_instruction()
            gate = gate.control(len(self.controls))
+            gate.control()


Is this call intentional?

Removed 330e580

hJaffaliColibritd · 2026-07-04T14:40:51Z

-            return UnitaryGate(self.matrix)
+                dummy_circuit.id(0)
+                return dummy_circuit.to_gate(
+                    label=f"CustomGate({', '.join([str(s) for s in gate_symbols])})"


Why did we change to this? the previous way of generating the label was not taking into account all the gate symbols?

Yes I think the issue was that not all the params were displayed.

Is there a risk, when we have too much parameters, that the display become weird ? in that case we can put "..." and not list them all but the first and last.
If this never happens you can resolve this comment

hJaffaliColibritd · 2026-07-06T07:39:51Z


-        return quantum_shannon_decomposition(self.matrix)
+        # non ordered targets
+        if any(


raise a warning or something in this case?

Is it necessary to raise a warning if the edge case is taken into account anyway ?

It's taken into account by tweaking the user's input, which may affect few stuff between what he submitted and what is actually run on the backend, right?
In that case it is nice if we can inform, or warn the user of the changes we make.

MathieuG-Colibri added 2 commits May 20, 2026 14:52

feat: reworked to_other_language and cirq custom gate

5d4e9c4

fix: tests

658d54e

github-actions Bot added the enhancement New feature or request label May 20, 2026

MathieuG-Colibri added 3 commits June 16, 2026 10:18

merge dev

8211fe6

feat: doc and style fixing

dd33f7a

feat: changed cirq from other language

d885b8e

github-actions Bot added the testing label Jun 16, 2026

MathieuG-Colibri marked this pull request as ready for review June 17, 2026 08:25

MathieuG-Colibri self-assigned this Jun 17, 2026

Henri-ColibrITD reviewed Jun 19, 2026

View reviewed changes

Comment thread mpqp/tools/unitary_decomposition.py Outdated

Henri-ColibrITD Jun 19, 2026

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.

not part of the PR but not super clear

Henri-ColibrITD reviewed Jun 19, 2026

View reviewed changes

Comment thread tests/execution/test_validity.py Outdated

Henri-ColibrITD reviewed Jun 19, 2026

View reviewed changes

Comment thread tests/execution/test_validity.py

Henri-ColibrITD reviewed Jun 19, 2026

View reviewed changes

Henri-ColibrITD requested changes Jun 19, 2026

View reviewed changes

MathieuG-Colibri and others added 5 commits June 25, 2026 13:57

Apply suggestions from code review

3e4073d

Co-authored-by: Henri de Boutray <133855265+Henri-ColibrITD@users.noreply.github.com>

fix: cleanup & refactoring

df88af6

fix: added test + cleanup

20f7d5c

fix: fixed documentation

2528b64

fix: typecheck

429c8eb

MoHermes self-requested a review July 1, 2026 09:24

MoHermes reviewed Jul 1, 2026

View reviewed changes

MoHermes requested changes Jul 1, 2026

View reviewed changes

hJaffaliColibritd reviewed Jul 4, 2026

View reviewed changes

hJaffaliColibritd reviewed Jul 6, 2026

View reviewed changes

fix: unnecesary call removed

330e580

Uh oh!

Conversation

MathieuG-Colibri commented May 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hJaffaliColibritd Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hJaffaliColibritd Jul 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

hJaffaliColibritd Jul 7, 2026 •

edited

Loading

hJaffaliColibritd Jul 4, 2026 •

edited

Loading