-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add library of gates with their equivalent circuit implementations. #3946
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks good
qiskit/circuit/equivalence.py
Outdated
"""Create a new equivalence library. | ||
|
||
Args: | ||
base - Base equivalence library to will be referenced if an entry is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
base - Base equivalence library to will be referenced if an entry is | |
base - Base equivalence library to which will be referenced if an entry is |
self.assertTrue( | ||
(entry[0] == first_equiv and entry[1] == second_equiv) | ||
or (entry[1] == first_equiv and entry[0] == second_equiv)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe compare sets?
self.assertTrue( | |
(entry[0] == first_equiv and entry[1] == second_equiv) | |
or (entry[1] == first_equiv and entry[0] == second_equiv)) | |
self.assertEqual(set(entry), set([first_equiv, second_equiv]) |
Why not return them in order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was on the fence about what the ordering should be. Ideas? (Insertion order oldest to newest from self to base is what we'll get now, I think.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my original comments was for being independent of the order. But I see value in keeping some deterministic order. That might be a proto-way to define priorities. I think entry[0] == newest
... maybe a stack?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would've preferred a set as well (for the return type and the assertion) but QuantumCircuit
s aren't hashable (as they're mutable). Agree deterministic order is good, but the caller should inspect the returned circuit to determine priority, so the exact order shouldn't be so important. I'll document the current order and remove the or
s in the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 7378cec .
I'm not fully sure if this works or should work... eq_lib = EquivalenceLibrary()
theta = Parameter('theta')
phi = Parameter('phi')
gate = OneQubitTwoParamGate(theta, phi)
equiv1 = QuantumCircuit(1)
equiv1.u2(theta, phi, 0) # theta,phi
eq_lib.add_equivalence(gate, equiv1)
equiv2 = QuantumCircuit(1)
equiv2.u2(phi, theta, 0) # phi,theta
eq_lib.add_equivalence(gate, equiv2) |
This will work now, but maybe the behavior isn't obvious. What would you expect in this case?
The above is saying that the gate is arg symmetric (both U2(a,b) and U2(b,a) implement 1q2p(a,b)). I can make it clearer in the docs that the mapping between the parameters of the gate and the paramters in the circuit is what is preserved in the library (and what will be reflected between the query gate and the returned circuits.) |
62869fc
to
76fc079
Compare
yes this sounds like a good behavior. |
76fc079
to
419a04c
Compare
…iskit#3946) * Add library of gates with their equivalent circuit implementations. * Document equivalent circuit return order. Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Summary
Adds EquivalenceLibrary structure from Qiskit/RFCs#6 .
Details and comments