Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Qiskit v0.20 #101

Merged
merged 3 commits into from
Aug 12, 2020
Merged

Qiskit v0.20 #101

merged 3 commits into from
Aug 12, 2020

Conversation

antalszava
Copy link
Contributor

@antalszava antalszava commented Aug 11, 2020

  • Removes the use of the CU1 gate in a test which tests for unsupported instructions. The CU1 gate is now supported with Qiskit v0.20 and is converted using its to_matrix method and QubitUnitary.
  • Monkeypatches the IBMQX_TOKEN environment variable in the test_account_error case (env var might be defined making the test fail)
  • Pins Qiskit to v0.20 in requirements.txt and setup.py

@codecov
Copy link

codecov bot commented Aug 11, 2020

Codecov Report

Merging #101 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #101   +/-   ##
=======================================
  Coverage   99.23%   99.23%           
=======================================
  Files           7        7           
  Lines         263      263           
=======================================
  Hits          261      261           
  Misses          2        2           

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 396094a...70ca150. Read the comment docs.

@@ -812,16 +806,13 @@ def test_qasm_from_file(self, tmpdir, recorder):
assert recorder.queue[5].parameters == []
assert recorder.queue[5].wires == Wires([3])

assert len(record) == 11
assert len(record) == 5
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I recall that the CU1Gate was added at the time because it was the only gate that lacked the to_matrix method. Hence it couldn't be converted.

Copy link
Member

Choose a reason for hiding this comment

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

Nice! So it turns out the test was failing because the converter now works automatically for the CU1 gate 😆

@antalszava antalszava requested review from josh146 and thisac August 11, 2020 18:55
Copy link
Contributor

@thisac thisac left a comment

Choose a reason for hiding this comment

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

I think this looks fine @antalszava. Not exactly sure how removing the CU1Gate impacts the tests, but it looks alright, and the tests seem to pass fine! 😄

Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

This is great, thanks @antalszava!

@thisac, I believe the CU1 gate is being automatically converted into a qml.QubitUnitary gate, via the qiskit to_matrix() method (which it previously didn't support, so a warning would be raised instead). As a result, the number of operations on the recorder increased, while the number of warning messages on the record decreased.

@@ -812,16 +806,13 @@ def test_qasm_from_file(self, tmpdir, recorder):
assert recorder.queue[5].parameters == []
assert recorder.queue[5].wires == Wires([3])

assert len(record) == 11
assert len(record) == 5
Copy link
Member

Choose a reason for hiding this comment

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

Nice! So it turns out the test was failing because the converter now works automatically for the CU1 gate 😆

Comment on lines +121 to +123
with monkeypatch.context() as m:
m.delenv("IBMQX_TOKEN", raising=False)
IBMQDevice(wires=1)
Copy link
Member

Choose a reason for hiding this comment

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

🎉

@josh146 josh146 merged commit ca61424 into master Aug 12, 2020
@josh146 josh146 deleted the qiskit_v0.20 branch August 12, 2020 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants