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.18 updates #81

Merged
merged 5 commits into from
Apr 15, 2020
Merged

Qiskit v0.18 updates #81

merged 5 commits into from
Apr 15, 2020

Conversation

antalszava
Copy link
Contributor

@antalszava antalszava commented Apr 14, 2020

With the new Qiskit version (0.18) certain changes were introduced affecting PennyLane-Qiskit as well.

List of changes:

C++/Python conversion

The following RuntimeError is propagated from Qiskit-Aer:

RuntimeError: Unable to cast Python instance to C++ type (compile in debug mode for details)

A suggested quick fix was used for now.

More details can be found here:
Qiskit/qiskit-aer#692

Updated names for some gates

Further renames:

  • ToffoliGate -> CCXGate
  • FredkinGate -> CSwapGate
  • Cu1Gate -> CU1Gate

…ng to list on the numpy parameters due to a C++ conversion bug in Qiskit Aer
…led rotation test as well as a CRY specific one
@codecov
Copy link

codecov bot commented Apr 14, 2020

Codecov Report

Merging #81 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #81   +/-   ##
=======================================
  Coverage   99.11%   99.12%           
=======================================
  Files           7        7           
  Lines         340      341    +1     
=======================================
+ Hits          337      338    +1     
  Misses          3        3           
Impacted Files Coverage Δ
pennylane_qiskit/qiskit_device.py 98.63% <100.00%> (+<0.01%) ⬆️

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 9b83966...bc6f99a. Read the comment docs.

@antalszava antalszava requested a review from josh146 April 14, 2020 23:22
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.

Awesome, thanks for catching this @antalszava!

Comment on lines +72 to +74
"CRX": ex.CRXGate,
"CRY": ex.CRYGate,
"CRZ": ex.CRZGate,
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

@@ -59,23 +59,24 @@ def pauli_eigs(n):
"PauliY": ex.YGate,
"PauliZ": ex.ZGate,
"Hadamard": ex.HGate,
"CNOT": ex.CnotGate,
"CZ": ex.CzGate,
"CNOT": ex.CXGate,
Copy link
Member

Choose a reason for hiding this comment

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

I wonder why they renamed this? CNOT is much more established than CX

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really good question. From what I saw they started porting every gate to this type of naming (e.g. FredkingGate was also renamed to CSwapGate) and I also noticed a base class for controlled X that was created.

They seem to not even mention CNOT with the new formalism (once the CnotGate has been deprecated, it will not be in there):
https://qiskit.org/documentation/_modules/qiskit/extensions/standard/x.html#CXGate

pennylane_qiskit/qiskit_device.py Outdated Show resolved Hide resolved
Comment on lines +259 to +260
"""Tests loading a circuit with two qubit controlled rotations (except
for CRY)."""
Copy link
Member

Choose a reason for hiding this comment

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

How come this test doesn't check CRY?

Comment on lines +281 to +283
# This test will be merged into the test_controlled_rotations test once
# the native CRY is used in Qiskit and not a set of instructions
# yielded from decomposition is being added
Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see!

Copy link
Member

Choose a reason for hiding this comment

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

How come this is the case? Is Qiskit doing the decomposition? Or PL core?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's coming from Qiskit, but the question of why it is so got me wondering as well 😅 :

from qiskit import QuantumCircuit, QuantumRegister

q2 = QuantumRegister(2)
qc = QuantumCircuit(q2)

instr_set = qc.cry(0.5, q2[0], q2[1])
instr_set.data

Outputs:

[(<qiskit.extensions.standard.u3.U3Gate object at 0x7fec26d88350>, [Qubit(QuantumRegister(2, 'q2'), 1)], []), (<qiskit.extensions.standard.x.CXGate object at 0x7fec26d88510>, [Qubit(QuantumRegister(2, 'q2'), 0), Qubit(QuantumRegister(2, 'q2'), 1)], []), (<qiskit.extensions.standard.u3.U3Gate object at 0x7fec26d885d0>, [Qubit(QuantumRegister(2, 'q2'), 1)], []), (<qiskit.extensions.standard.x.CXGate object at 0x7fec26d88610>, [Qubit(QuantumRegister(2, 'q2'), 0), Qubit(QuantumRegister(2, 'q2'), 1)], [])]

@josh146 josh146 merged commit e56491c into master Apr 15, 2020
@josh146 josh146 deleted the v0.18_updates branch April 15, 2020 02:42
This was referenced Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants