Fix wrong relative phase of MCRZ #9836

adjs · 2023-03-22T02:10:14Z

Summary

This PR is on top of PR #9688 and fixes #9202.

Co-authored-by: thiagom123 <thiagomdazevedo@hotmail.com> Co-authored-by: IsmaelCesar <leamscesar@gmail.com> Co-authored-by: Israel F. Araujo <israelferrazaraujo@hotmail.com> Co-authored-by: Adenilton Silva <7927558+adjs@users.noreply.github.com>

…into mcsu2

This reverts commit c1ceaf6.

This reverts commit 7c75611.

This reverts commit 100a690.

This reverts commit b762b39.

This reverts commit fab1c80.

adjs · 2023-04-21T17:49:51Z

Why don't the gates mcrx, mcry and mcrz appear in the circuit library?
https://qiskit.org/documentation/apidoc/circuit_library.html
Should they be added to the init file?
https://github.com/Qiskit/qiskit-terra/blob/main/qiskit/circuit/library/__init__.py

Why do the mcry gate have a q_ancillae parameter and mcrx and mcrz do not have it?

I think this could be discussed in another PR, because these points are also present in the main branch.

adjs · 2023-04-21T17:52:54Z

Thanks for this addition, overall the fix looks great! I left some comments below, mainly formalities.

If possible I think we would like to include this in the upcoming release, so let us know if there's anything we can help you with 🙂

./run_experiments doesn't work on my computer and I couldn't find documentation on these tests. Could you fix the compatibility tests?

adjs · 2023-04-21T17:53:48Z

@ShellyGarion and @Cryoris thanks for the review.

mtreinish · 2023-05-01T20:11:31Z

I looked at the qpy test failures locally (fwiw all you need is to have gnu parallel installed and run ./run_tests.sh from the test/qpy_compat dir) the root cause of the failure is the changes to the generic .control() definition. This is causing a divergence from the nested definition of the custom controlled gate from the qpy files generate in the old release and the expected circuit we generate in the current main version of the code. I think if we're saying the definition prior to this PR was incorrect and needs to be different the only path forward here is to change https://github.com/Qiskit/qiskit-terra/blob/main/test/qpy_compat/test_qpy.py#L605-L610 to only run generate_controlled_gates() and generate_open_controlled_gates() if version_parts >= (0, 25, 0) because we're not going to be able to validate the equivalence of the stored qpy prior to this change otherwise.

Cryoris · 2023-05-03T12:11:19Z

qiskit/circuit/library/standard_gates/multi_control_rotation_gates.py

+        # pylint: disable=cyclic-import
+        from qiskit import transpile
+
+        self = transpile(self, basis_gates=["p", "u", "cx"], optimization_level=0)


I don't think this is quite correct, sorry if I didn't express it correctly before. With this code, the whole circuit will get transpiled to the specified basis -- but we only want the newly added mcrz to be in that basis 🙂 maybe we can just change mcsu2_real_diagonal to return the MCRZ subcircuit and then transpile that?

Hi @Cryoris , I removed the transpile call.
I think that we should not transpile the MCRZ circuit here, because of the transpile cost with a higher number of controls.

I agree it's not the most efficient way, but we should ensure that use_basis_gates=True will still behave as it did before 🙂 If we want to avoid transpiling, would it be possible to express mcsu2 in the P U CX basis?

Cryoris · 2023-05-16T14:30:16Z

I pushed a small change to accomodate the use_basis_gates, other than that the PR LGTM. Since I modified the code maybe @mtreinish, @ShellyGarion or @alexanderivrii could have another look?

adjs · 2023-05-18T11:39:28Z

Thanks @Cryoris. Your modifications LGTM.

mtreinish

LGTM too

* efficient multicontrolled su2 gate decomposition Co-authored-by: thiagom123 <thiagomdazevedo@hotmail.com> Co-authored-by: IsmaelCesar <leamscesar@gmail.com> Co-authored-by: Israel F. Araujo <israelferrazaraujo@hotmail.com> Co-authored-by: Adenilton Silva <7927558+adjs@users.noreply.github.com> * removed optimization flag Co-authored-by: thiagom123 <thiagomdazevedo@hotmail.com> Co-authored-by: IsmaelCesar <leamscesar@gmail.com> Co-authored-by: Israel F. Araujo <israelferrazaraujo@hotmail.com> Co-authored-by: Adenilton Silva <7927558+adjs@users.noreply.github.com> * tox -eblack * updated docstrings * Adds `MCSU2Gate` to `__init__` * fixed circular import * defined control and inverse methods * changed MCSU2Gate from Gate to ControlledGate * adjusted some tests for controlled gates * reformatting * Fix regarding the integer `ctrl_state` parameter * Tests to check the CX count upper bound * Gate's `label` in the `control` function * Upd. Qiskit tests to include cases for MCSU2Gate * Upd. Qiskit tests to include cases for MCSU2Gate * Revert "Upd. Qiskit tests to include cases for MCSU2Gate" This reverts commit c1ceaf6. * Revert "Upd. Qiskit tests to include cases for MCSU2Gate" This reverts commit 7c75611. * Revert "Tests to check the CX count upper bound" This reverts commit 100a690. * Update test_controlled_gate.py * Update test_circuit_operations.py * remove mcsu2gate class * remove mcsu2gate class * fix mcry * lint * fix mcrx * add reference * Create `s_gate` directly * Revert "Create `s_gate` directly" This reverts commit b762b39. * review * release notes * review 2 * backwards compat * function signature and number of controls * fix mcrz * Update multi_control_rotation_gates.py * review * Update test_qpy.py * Revert "Update test_qpy.py" This reverts commit fab1c80. * Update test_qpy.py * Update multi_control_rotation_gates.py * Fix `use_basis_gates=True` case * lint --------- Co-authored-by: rafaella-vale <26910380+rafaella-vale@users.noreply.github.com> Co-authored-by: thiagom123 <thiagomdazevedo@hotmail.com> Co-authored-by: IsmaelCesar <leamscesar@gmail.com> Co-authored-by: Israel F. Araujo <israelferrazaraujo@hotmail.com> Co-authored-by: Rafaella Vale <rfv@cin.ufpe.br> Co-authored-by: Julien Gacon <gaconju@gmail.com> (cherry picked from commit 7b677ed) # Conflicts: # qiskit/circuit/library/standard_gates/multi_control_rotation_gates.py

* Fix wrong relative phase of MCRZ (#9836) * efficient multicontrolled su2 gate decomposition Co-authored-by: thiagom123 <thiagomdazevedo@hotmail.com> Co-authored-by: IsmaelCesar <leamscesar@gmail.com> Co-authored-by: Israel F. Araujo <israelferrazaraujo@hotmail.com> Co-authored-by: Adenilton Silva <7927558+adjs@users.noreply.github.com> * removed optimization flag Co-authored-by: thiagom123 <thiagomdazevedo@hotmail.com> Co-authored-by: IsmaelCesar <leamscesar@gmail.com> Co-authored-by: Israel F. Araujo <israelferrazaraujo@hotmail.com> Co-authored-by: Adenilton Silva <7927558+adjs@users.noreply.github.com> * tox -eblack * updated docstrings * Adds `MCSU2Gate` to `__init__` * fixed circular import * defined control and inverse methods * changed MCSU2Gate from Gate to ControlledGate * adjusted some tests for controlled gates * reformatting * Fix regarding the integer `ctrl_state` parameter * Tests to check the CX count upper bound * Gate's `label` in the `control` function * Upd. Qiskit tests to include cases for MCSU2Gate * Upd. Qiskit tests to include cases for MCSU2Gate * Revert "Upd. Qiskit tests to include cases for MCSU2Gate" This reverts commit c1ceaf6. * Revert "Upd. Qiskit tests to include cases for MCSU2Gate" This reverts commit 7c75611. * Revert "Tests to check the CX count upper bound" This reverts commit 100a690. * Update test_controlled_gate.py * Update test_circuit_operations.py * remove mcsu2gate class * remove mcsu2gate class * fix mcry * lint * fix mcrx * add reference * Create `s_gate` directly * Revert "Create `s_gate` directly" This reverts commit b762b39. * review * release notes * review 2 * backwards compat * function signature and number of controls * fix mcrz * Update multi_control_rotation_gates.py * review * Update test_qpy.py * Revert "Update test_qpy.py" This reverts commit fab1c80. * Update test_qpy.py * Update multi_control_rotation_gates.py * Fix `use_basis_gates=True` case * lint --------- Co-authored-by: rafaella-vale <26910380+rafaella-vale@users.noreply.github.com> Co-authored-by: thiagom123 <thiagomdazevedo@hotmail.com> Co-authored-by: IsmaelCesar <leamscesar@gmail.com> Co-authored-by: Israel F. Araujo <israelferrazaraujo@hotmail.com> Co-authored-by: Rafaella Vale <rfv@cin.ufpe.br> Co-authored-by: Julien Gacon <gaconju@gmail.com> (cherry picked from commit 7b677ed) # Conflicts: # qiskit/circuit/library/standard_gates/multi_control_rotation_gates.py * fix conflicts --------- Co-authored-by: Adenilton Silva <7927558+adjs@users.noreply.github.com> Co-authored-by: Julien Gacon <gaconju@gmail.com>

rafaella-vale and others added 30 commits March 1, 2023 01:04

removed optimization flag

2c81328

Co-authored-by: thiagom123 <thiagomdazevedo@hotmail.com> Co-authored-by: IsmaelCesar <leamscesar@gmail.com> Co-authored-by: Israel F. Araujo <israelferrazaraujo@hotmail.com> Co-authored-by: Adenilton Silva <7927558+adjs@users.noreply.github.com>

tox -eblack

a8fbb81

updated docstrings

5151a42

Adds MCSU2Gate to __init__

0aec4ec

Merge branch 'mcsu2' of https://github.com/comp-quantica/qiskit-terra …

b01a093

…into mcsu2

fixed circular import

e39e09d

defined control and inverse methods

d632819

changed MCSU2Gate from Gate to ControlledGate

32acacc

adjusted some tests for controlled gates

3c8bdc4

reformatting

3a92dc9

Merge branch 'main' into mcsu2

155a982

Fix regarding the integer ctrl_state parameter

ed6953b

Tests to check the CX count upper bound

100a690

Gate's label in the control function

7d3ea14

Upd. Qiskit tests to include cases for MCSU2Gate

7c75611

Upd. Qiskit tests to include cases for MCSU2Gate

c1ceaf6

Revert "Upd. Qiskit tests to include cases for MCSU2Gate"

39d35b4

This reverts commit c1ceaf6.

Revert "Upd. Qiskit tests to include cases for MCSU2Gate"

c00d17a

This reverts commit 7c75611.

Revert "Tests to check the CX count upper bound"

0a6148d

This reverts commit 100a690.

Update test_controlled_gate.py

f4042c1

Update test_circuit_operations.py

b999c20

remove mcsu2gate class

0c3eeed

remove mcsu2gate class

6bdb533

fix mcry

9b02451

lint

2568a39

fix mcrx

15034fd

add reference

fe874ef

Create s_gate directly

b762b39

Revert "Create s_gate directly"

d75b9c9

This reverts commit b762b39.

Revert "Update test_qpy.py"

3aedea6

This reverts commit fab1c80.

adjs requested a review from Cryoris April 21, 2023 17:54

kdk assigned mtreinish May 1, 2023

mtreinish added the Changelog: Bugfix Include in the "Fixed" section of the changelog label May 1, 2023

Cryoris reviewed May 3, 2023

View reviewed changes

mtreinish added the priority: medium label May 3, 2023

adjs added 2 commits May 3, 2023 15:11

Update test_qpy.py

fd14c87

Merge branch 'main' into fix-mcrz

856cd15

mtreinish modified the milestones: 0.24.0, 0.24.1 May 4, 2023

Update multi_control_rotation_gates.py

7a9df50

adjs requested a review from Cryoris May 4, 2023 21:01

Cryoris added 2 commits May 16, 2023 15:53

Fix use_basis_gates=True case

9269ecf

lint

fb180f4

mtreinish approved these changes May 22, 2023

View reviewed changes

mtreinish added this pull request to the merge queue May 22, 2023

Merged via the queue into Qiskit:main with commit 7b677ed May 22, 2023

mergify bot mentioned this pull request May 22, 2023

Fix wrong relative phase of MCRZ (backport #9836) #10141

Merged

ShellyGarion mentioned this pull request May 25, 2023

Multicontrol Rz fix #9574

Closed

adjs deleted the fix-mcrz branch August 2, 2023 18:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix wrong relative phase of MCRZ #9836

Fix wrong relative phase of MCRZ #9836

adjs commented Mar 22, 2023 •

edited by kdk

Loading

adjs commented Apr 21, 2023 •

edited

Loading

adjs commented Apr 21, 2023 •

edited

Loading

adjs commented Apr 21, 2023

mtreinish commented May 1, 2023

Cryoris May 3, 2023

adjs May 4, 2023

Cryoris May 9, 2023

Cryoris commented May 16, 2023 •

edited

Loading

adjs commented May 18, 2023

mtreinish left a comment

Fix wrong relative phase of MCRZ #9836

Fix wrong relative phase of MCRZ #9836

Conversation

adjs commented Mar 22, 2023 • edited by kdk Loading

Summary

adjs commented Apr 21, 2023 • edited Loading

adjs commented Apr 21, 2023 • edited Loading

adjs commented Apr 21, 2023

mtreinish commented May 1, 2023

Cryoris May 3, 2023

Choose a reason for hiding this comment

adjs May 4, 2023

Choose a reason for hiding this comment

Cryoris May 9, 2023

Choose a reason for hiding this comment

Cryoris commented May 16, 2023 • edited Loading

adjs commented May 18, 2023

mtreinish left a comment

Choose a reason for hiding this comment

adjs commented Mar 22, 2023 •

edited by kdk

Loading

adjs commented Apr 21, 2023 •

edited

Loading

adjs commented Apr 21, 2023 •

edited

Loading

Cryoris commented May 16, 2023 •

edited

Loading