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
qutip-qip backends for qiskit #155
Conversation
2184a1a
to
b71f236
Compare
0bd7492
to
b708d10
Compare
src/qutip_qip/qiskit/backend.py
Outdated
if statistics.cbits[0] is not None: | ||
for i, count in enumerate(statistics.cbits): | ||
counts[convert_to_hex(count)] = round( | ||
statistics.probabilities[i], 3 | ||
) |
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.
Why round it here?
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.
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 is better to keep the default as not rounded. Although the rounded one looks cleaner, it unnecessarily makes the result inaccurate. If we do want a better display, we should only round it when displaying, not the original result. But I guess .data()
is meant to be raw data and one can define some .probability
for nice looking and give it some parameters e.g. truncation
.
BTW, why does counts
returns a probability... I know that we don't repeat. So we probably don't have a well-defined counts
? How does the qiskit circuit simulator handle this?
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.
Qiskit backends have an option to set the number of shots
. I haven't implemented any configurable backend options yet and will be doing that soon. After doing that, the probabilities will be replaced by the actual counts
. We can use random sampling from the obtained probabilities for this purpose.
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.
Looks good. The main comment is about the warning. Otherwise, I think it is ready!
src/qutip_qip/qiskit/converter.py
Outdated
_ignore_gates = ["id", "barrier"] | ||
|
||
|
||
def _make_user_gate(unitary: np.ndarray, instruction: Instruction): |
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.
Let's make it clear that it is the qiskit Instracution, qutip
also has its own Instruction
.
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.
Looks good
Seems that codeclimate is undergoing some connection error. I'll just merge. |
This draft PR tracks changes for the GSoC '22 project: making qutip-qip backends for qiskit.