Bump cirq version from `0.10.0` to `0.13.0` #988

vtomole · 2021-10-17T00:34:28Z

Replacing #915

Description

Checklist

Check off the following once complete (or if not applicable) after opening the PR. The PR will be reviewed once this checklist is complete and all tests are passing.

I added unit tests for new code.
I used type hints in function signatures.
I used Google-style docstrings for functions.
I updated the documentation where relevant.

If some items remain, you can mark this a draft pull request.

Tips

If the validation check fails:
1. Run make check-types (from the root directory of the repository) and fix any mypy errors.
2. Run make check-style and fix any flake8 errors.
3. Run make format to format your code with the black autoformatter.
For more information, check the Mitiq style guidelines.
Write "Fixes #XYZ" in the description if this PR fixes Issue #XYZ.

Replacing unitaryfund#915

github-actions · 2021-10-17T00:34:39Z

👈 Launch a binder notebook on branch vtomole/mitiq/patch-1

github-actions

Hello @vtomole, thank you for submitting a PR to Mitiq! We will respond as soon as possible, and if you have any questions in the meantime, you can ask us on the Unitary Fund Discord.

vtomole · 2021-10-17T00:38:35Z

This change depends on either #927 or #935 for the checks to pass.

rmlarose · 2021-10-19T03:35:56Z

Thanks @vtomole. Can you merge master? #992 handled pyquil. Should be good to go after this.

vtomole · 2021-10-19T04:03:08Z

@rmlarose Done.

nathanshammah · 2021-10-19T10:40:47Z

Thank you @vtomole. At validation there are these errors

mitiq/pec/representations/optimal.py:23: error: Module "cirq" has no attribute "channel"  [attr-defined]
mitiq/pec/representations/depolarizing.py:22: error: Module "cirq" has no attribute "channel"  [attr-defined]
mitiq/pec/representations/damping.py:22: error: Module "cirq" has no attribute "channel"  [attr-defined]
Found 3 errors in 3 files (checked 53 source files)

I could not track where channel ended up into in the current Cirq.

There are also these issues in tests with pennylane and PEC:

=========================== short test summary info ============================
FAILED mitiq/interface/mitiq_pennylane/tests/test_conversions_pennylane.py::test_integration
FAILED mitiq/cdr/tests/test_clifford_training_data.py::test_count_non_cliffords[pennylane]
ERROR mitiq/pec/representations/tests/test_damping.py
ERROR mitiq/pec/representations/tests/test_depolarizing.py
ERROR mitiq/pec/representations/tests/test_optimal.py
ERROR mitiq/pec/tests/test_channels.py
ERROR mitiq/pec/tests/test_pec.py
ERROR mitiq/pec/tests/test_pec_sampling.py
====== 2 failed, 984 passed, 124 warnings, 6 errors in 364.11s (0:06:04) =======
make: *** [Makefile:75: test-all] Error 1

vtomole · 2021-10-19T15:13:27Z

cirq.channel was deprecated. I've pushed the updated changes.

andreamari · 2021-10-20T15:40:44Z

@vtomole Thanks!
From the log of the tests, it seems that there are other cirq.channels around to be updated to cirq.kraus. Probably also in the PEC example of the docs.

There are also other strange errors on PennyLane tests that I don't immediately understand (maybe Cirq changed something about qasm conversions 🤔).

vtomole · 2021-10-26T01:14:21Z

quantumlib/Cirq#4113 breaks the pennylane conversion to a point where

a, b = cirq.LineQubit.range(2)
    circuit = Circuit(
        cirq.rz(0.0).on(a),  # Clifford.
        cirq.rx(0.1 * np.pi).on(b),  # Non-Clifford.
        cirq.rx(0.5 * np.pi).on(b),  # Clifford
        cirq.rz(0.4 * np.pi).on(b),  # Non-Clifford.
        cirq.rz(0.5 * np.pi).on(b),  # Clifford.
        cirq.CNOT.on(a, b),  # Clifford.
    )

compiles to

cirq.Circuit(
        cirq.Rz(0.0).on(cirq.NamedQubit('q_0')),
        cirq.Rx(0.3141592653589793).on(cirq.NamedQubit('q_1')),

        cirq.Rz(1.5707963267948963).on(cirq.NamedQubit('q_1')),
        cirq.Ry(1.5707963267948968).on(cirq.NamedQubit('q_1')),
    
        cirq.Rz(-1.5707963267948966).on(cirq.NamedQubit('q_1')),
        cirq.Rz(1.2566370614359172).on(cirq.NamedQubit('q_1')),
        cirq.circuits.qasm_output.QasmUGate(theta=0, phi=0, lmda=0.5).on(cirq.NamedQubit('q_1')),
        cirq.CNOT(cirq.NamedQubit('q_0'), cirq.NamedQubit('q_1')),
        )

Notice the cirq.circuits.qasm_output.QasmUGate(theta=0, phi=0, lmda=0.5).on(cirq.NamedQubit('q_1')), gate.

nathanshammah · 2021-11-11T09:48:04Z

Thanks for looking in detail into the breaking issue, @vtomole.

vtomole · 2021-11-11T15:38:26Z

@nathanshammah What do you think is best way to fix this? I think we should raise this issue up to pennylane because pennylane_from_qasm

mitiq/mitiq/interface/mitiq_pennylane/conversions.py

Line 84 in 8a04147

qfunc = pennylane_from_qasm(to_qasm(circuit))

is giving the incorrect conversion.

vtomole · 2021-11-11T16:19:46Z

Pennylane doesn't support sx gate out of the box. It compiles it to a QubitUnitary instead

import pennylane

qasm_string = """
// Generated from Cirq v0.13.1

OPENQASM 2.0;
include "qelib1.inc";


// Qubits: [0, 1]
qreg q[2];


rz(0) q[0];
rx(pi*0.1) q[1];
sx q[1];
rz(pi*0.4) q[1];
s q[1];
cx q[0],q[1];
"""

qfunc = pennylane.from_qasm(qasm_string)

with pennylane.tape.QuantumTape() as tape:
        qfunc(wires=pennylane.wires.Wires(range(2)))

print(tape.operations)
# prints
# [RZ(0.0, wires=[0]), 
# RX(0.3141592653589793, wires=[1]), 
# QubitUnitary(array([[0.5+0.5j, 0.5-0.5j],
#        [0.5-0.5j, 0.5+0.5j]]), wires=[1]), 
#        RZ(1.2566370614359172, wires=[1]), 
#        S(wires=[1]), 
#        CNOT(wires=[0, 1])]

Just moved to PennyLaneAI/pennylane#1891

vtomole · 2021-11-11T16:27:21Z

Not a solution; but to get this PR in we could remove the cirq.rx(0.5 * np.pi).on(b) operation, # Clifford out of this test.

vtomole · 2021-11-12T00:17:04Z

The fix: PennyLaneAI/pennylane-qiskit#158

andreamari · 2021-11-12T08:31:56Z

The fix: PennyLaneAI/pennylane-qiskit#158

Thanks for asking for this fix in PL, @vtomole! Given this fact, I think we just need to wait.

nathanshammah · 2021-11-12T15:36:23Z

Great suggestion, @vtomole!

vtomole · 2021-11-12T15:47:17Z

Given this fact, I think we just need to wait.

Great suggestion

I'm getting conflicting information. Should we wait for that Pennylane PR to get merged and for Pennylane to cut a new release before we merge this or should we modify the test here so it passes for us?

nathanshammah · 2021-11-12T15:51:02Z

@vtomole I referred to the idea of upstreaming this to Pennylane and the issue you opened on Pennylane. Let's wait for that release. We can also discuss at the Mitiq meeting if you want to join, at 12pm ET on http://discord.unitary.fund/

vtomole · 2021-11-12T15:55:18Z

Okay, thanks for clearing that up!

vtomole · 2021-12-14T19:09:44Z

@rmlarose @nathanshammah @rmlarose Ready for re-review 🙂

nathanshammah · 2021-12-15T18:54:25Z

Probably this will have the same issue of #1048(?), the pennylane update. #1049 updates pennylane-qiskit.

vtomole · 2022-01-27T17:08:52Z

The docs check is failing with a

Command killed due to excessive memory consumption

message.

crazy4pi314 · 2022-01-27T17:50:18Z

The docs check is failing with a

Command killed due to excessive memory consumption

message.

This can randomly happen when one of the examples has a call to scipy.fit that does not converge, often re-running (which I just kicked off) should allow it to proceed if that was the issue.

codecov · 2022-01-27T18:02:44Z

Codecov Report

Merging #988 (851d7c3) into master (84385d9) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #988   +/-   ##
=======================================
  Coverage   97.98%   97.98%           
=======================================
  Files          54       54           
  Lines        2527     2527           
=======================================
  Hits         2476     2476           
  Misses         51       51

Impacted Files	Coverage Δ
mitiq/pec/representations/damping.py	`100.00% <100.00%> (ø)`
mitiq/pec/representations/depolarizing.py	`100.00% <100.00%> (ø)`
mitiq/pec/representations/optimal.py	`97.22% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84385d9...851d7c3. Read the comment docs.

vtomole · 2022-01-27T19:50:50Z

@nathanshammah @andreamari Ready for re-review :)

mitiq/interface/mitiq_pennylane/tests/test_conversions_pennylane.py

andreamari · 2022-01-28T13:53:27Z

mitiq/interface/mitiq_pennylane/tests/test_conversions_pennylane.py

    u_1 = cirq.unitary(base_circ)
    u_2 = cirq.unitary(circ_recovered)
-
-    assert np.allclose(u_1, u_2)
+    cirq.testing.assert_allclose_up_to_global_phase(u_1, u_2, atol=0)


andreamari · 2022-01-28T14:02:00Z

mitiq/interface/mitiq_pennylane/tests/test_conversions_pennylane.py

-                params = list(np.pi * np.random.rand(gate.num_params))
-                rnd_wires = np.random.choice(
-                    range(n_wires), size=gate.num_wires, replace=False
-                )
-                gate(
-                    *params,
-                    wires=[
-                        int(w) for w in rnd_wires
-                    ],  # make sure we do not address wires as 0-d arrays
-                )


I guess you had to remove this for some tricky reason.
If not possible to re-introduce:

Can we restore the random rotation parameters only?

Can we restore the random wires only?

If not, it's fine. This is not a blocker for this PR.

The reason for this change is because gate.num_params is now dynamic. Since this test is copied from pennylane, the following PR recommends re-writing the test this way. See https://github.com/PennyLaneAI/pennylane/pull/1898/files#r750946650.

Ah, I see, very nice finding!

…ne.py Co-authored-by: Andrea Mari <[email protected]>

andreamari

Many thanks @vtomole! LGTM
(running all the tests now, we can merge once they pass)

nathanshammah · 2022-01-31T18:15:45Z

Thanks @vtomole! LGTM. I opened #1112 to check that RTD does not fail on all PRs. I will restart the checks to see if now they pass. It was killed for eccessive memory consumption.

andreamari · 2022-02-01T11:29:41Z

Hi @astrojuanlu, do you know why RDT is failing for this PR (and only for this PR)?

My guess, please correct me if I am wrong, is this: due to a slightly longer pip install process, the docs build of this PR exceeds the 15 minutes limit that is discussed at this page (https://docs.readthedocs.io/en/stable/builds.html).
In the same page it is also written that:

We can increase build limits on a per-project basis. Send an email to [email protected] providing a good reason why your documentation needs more resources.

Do you think we can get some extra build time? This would help us unblock urgent PRs until we find a more scalable solution for the docs.

astrojuanlu · 2022-02-01T12:11:01Z

Hi @andreamari ! It looks like the other builds are on the edge of the 15 minute limit. Looks like most of the build time is spent executing notebooks, which makes a ton of sense. Please send an email to [email protected] and the support team will take care of it :)

nathanshammah · 2022-02-01T12:16:56Z

Thanks @astrojuanlu for the info and @andreamari for investigating.

If we switch in the future to Jupyter book, it may be simpler to cache notebooks (at least for some CI checks).

andreamari · 2022-02-01T13:45:13Z

Hi @andreamari ! It looks like the other builds are on the edge of the 15 minute limit. Looks like most of the build time is spent executing notebooks, which makes a ton of sense. Please send an email to [email protected] and the support team will take care of it :)

I just sent and email to [email protected], thanks for the advice!

nathanshammah

Thank you @vtomole, all tests now pass.

andreamari · 2022-02-02T13:28:30Z

And also thanks to @astrojuanlu (RTD is working now)!

vtomole · 2022-02-02T15:17:54Z

@nathanshammah @andreamari Thanks for your patience!

andreamari · 2022-02-14T14:20:49Z

@allcontributors please add @vtomole for tests and code

allcontributors · 2022-02-14T14:20:58Z

@andreamari

I've put up a pull request to add @vtomole! 🎉

Bump cirq version from 0.10.0 to 0.13.0

f5a21cb

Replacing unitaryfund#915

github-actions bot reviewed Oct 17, 2021

View reviewed changes

Merge branch 'unitaryfund:master' into patch-1

ac8a755

channel -> kraus

c553e85

vtomole added 2 commits October 22, 2021 15:36

Change other stuff to channel

33dd697

Change to kraus as well

e35c42d

vtomole added 3 commits December 14, 2021 11:57

Update dev_requirements.txt

bee340b

Merge branch 'master' into patch-1

30defc9

Update dev_requirements.txt

8f7cf6c

Merge branch 'master' into patch-1

56cd6af

Merge branch 'master' into patch-1

19345b3

vtomole added 2 commits January 27, 2022 12:11

Format

d9b05d4

Merge branch 'patch-1' of github.com:vtomole/mitiq into patch-1

40de008

crazy4pi314 requested review from andreamari and nathanshammah January 28, 2022 00:39

andreamari reviewed Jan 28, 2022

View reviewed changes

Update mitiq/interface/mitiq_pennylane/tests/test_conversions_pennyla…

d9a8cd1

…ne.py Co-authored-by: Andrea Mari <[email protected]>

andreamari assigned nathanshammah Jan 28, 2022

Merge branch 'master' into patch-1

851d7c3

andreamari self-requested a review January 31, 2022 09:58

andreamari approved these changes Jan 31, 2022

View reviewed changes

nathanshammah approved these changes Feb 2, 2022

View reviewed changes

nathanshammah merged commit b687150 into unitaryfund:master Feb 2, 2022

vtomole deleted the patch-1 branch February 2, 2022 15:17

andreamari mentioned this pull request Feb 2, 2022

RTD build test #1112

Closed

4 tasks

allcontributors bot mentioned this pull request Feb 14, 2022

docs: add vtomole as a contributor for test, code #1132

Merged

Bump cirq version from 0.10.0 to 0.13.0 #988

Bump cirq version from 0.10.0 to 0.13.0 #988

Conversation

vtomole commented Oct 17, 2021

Description

Checklist

Tips

github-actions bot commented Oct 17, 2021

github-actions bot left a comment

Choose a reason for hiding this comment

vtomole commented Oct 17, 2021 • edited Loading

rmlarose commented Oct 19, 2021

vtomole commented Oct 19, 2021

nathanshammah commented Oct 19, 2021 • edited Loading

vtomole commented Oct 19, 2021

andreamari commented Oct 20, 2021

vtomole commented Oct 26, 2021

nathanshammah commented Nov 11, 2021

vtomole commented Nov 11, 2021

vtomole commented Nov 11, 2021 • edited Loading

vtomole commented Nov 11, 2021

vtomole commented Nov 12, 2021

andreamari commented Nov 12, 2021 • edited Loading

nathanshammah commented Nov 12, 2021

vtomole commented Nov 12, 2021

nathanshammah commented Nov 12, 2021

vtomole commented Nov 12, 2021

vtomole commented Dec 14, 2021

nathanshammah commented Dec 15, 2021

vtomole commented Jan 27, 2022

crazy4pi314 commented Jan 27, 2022

codecov bot commented Jan 27, 2022 • edited Loading

Codecov Report

vtomole commented Jan 27, 2022

andreamari Jan 28, 2022

Choose a reason for hiding this comment

andreamari Jan 28, 2022

Choose a reason for hiding this comment

vtomole Jan 28, 2022

Choose a reason for hiding this comment

andreamari Jan 31, 2022

Choose a reason for hiding this comment

andreamari left a comment

Choose a reason for hiding this comment

nathanshammah commented Jan 31, 2022

andreamari commented Feb 1, 2022

astrojuanlu commented Feb 1, 2022

nathanshammah commented Feb 1, 2022

andreamari commented Feb 1, 2022 • edited Loading

nathanshammah left a comment

Choose a reason for hiding this comment

andreamari commented Feb 2, 2022

vtomole commented Feb 2, 2022

andreamari commented Feb 14, 2022

allcontributors bot commented Feb 14, 2022

Bump cirq version from `0.10.0` to `0.13.0` #988

Bump cirq version from `0.10.0` to `0.13.0` #988

vtomole commented Oct 17, 2021 •

edited

Loading

nathanshammah commented Oct 19, 2021 •

edited

Loading

vtomole commented Nov 11, 2021 •

edited

Loading

andreamari commented Nov 12, 2021 •

edited

Loading

codecov bot commented Jan 27, 2022 •

edited

Loading

andreamari commented Feb 1, 2022 •

edited

Loading