sandialabs / pyGSTi

A python implementation of Gate Set Tomography
http://www.pygsti.info
Apache License 2.0
136 stars 56 forks source link

ECR Gate Compatibility #440

Closed pcwysoc closed 1 month ago

pcwysoc commented 6 months ago

Is your feature request related to a problem? Please describe. Current code for interfacing with IBM devices is not compatible with the ECR gate. However, all but one of the available IBM devices uses the ECR gate.

Describe the solution you'd like I believe we should refactor the IBM code to convert directly from pyGSTi circuits to Qiskit circuits, as this would smoothly integrate the ECR gate.

Describe alternatives you've considered I've considered trying to convert ECR gates to OpenQASM 2.0 as we currently do. However, this leads to very fragile QASM code:

OPENQASM 2.0; include "qelib1.inc"; gate rzx(param0) q0,q1 { h q1; cx q0,q1; rz(param0) q1; cx q0,q1; h q1; } gate ecr q0,q1 { rzx(pi/4) q0,q1; x q0; rzx(-pi/4) q0,q1; } qreg q[3]; creg c[1]; ecr q[1],q[2]; ecr q[0],q[2]; measure q[2] -> c[0]; barrier q[0],q[1],q[2];

This works for jobs submitted through Runtime or the Quantum Composer on the IBM website, but seems to fail for jobs submitted through backend.run():

Screenshot 2024-05-15 at 12 41 16 PM

I'm concerned that if we pursue this avenue that the fix would be unstable.

jordanh6 commented 6 months ago

I'm popping in to note that I'd also find ECR compatibility helpful, as someone else working with IBM's devices currently.

I haven't thought about the best approach, but I'll note that if we decide to convert to Qiskit circuits we need to be careful to control how those circuits get transpiled by Qiskit before running (so that they don't get optimized/compiled in unintended ways).

coreyostrove commented 5 months ago

This works for jobs submitted through Runtime or the Quantum Composer on the IBM website, but seems to fail for jobs submitted through Runtime:

Question for you coming from my position of relative ignorance on the IBM integration code: Any idea if this is an 'us' problem or a 'them' problem? I.e. is this a bug in our code, or something we should be reporting on one of IBM's repos?

Edit: When you say Runtime the second time, do you mean when using the python API?

pcwysoc commented 5 months ago

@coreyostrove

I would say that it's an IBM/QASM suboptimality that is unlikely to be fixed and we are forced to accommodate. QASM does not directly allow a native ECR gate, i.e. writing

ecr q[0], q[1];

and having that compile to an ECR gate on qubits 0 and 1. IBM has attempted to work around this by defining its method for converting Qiskit circuits to QASM a macro:

gate rzx(param0) q0,q1 { h q1; cx q0,q1; rz(param0) q1; cx q0,q1; h q1; } gate ecr q0,q1 { rzx(pi/4) q0,q1; x q0; rzx(-pi/4) q0,q1; }

We currently convert PyGSTi circuits to QASM to Qiskit circuits (to avoid more than minimal compilation). If we:

  1. use this macro in the PyGSTi circuits to QASM code
  2. convert QASM to Qiskit circuits
  3. run via on IBM hardware using backend.run()

We get an error that the gate we implemented is not valid (I can't remember the exact error code--it's been a few months). If we run via pasting the QASM circuit into the IBM Quantum Composer it works. This shows how unstable the macro is (which also can be observed from the fact that none of the parameters or specifics defined in it matter). We talked to some folks from IBM about this at March Meeting and it doesn't seem to be a priority to fix.

PS: The second instance was a typo and has been corrected. It was intended to read backend.run().

coreyostrove commented 5 months ago

IBM has attempted to work around this by defining its method for converting Qiskit circuits to QASM a macro:

gate rzx(param0) q0,q1 { h q1; cx q0,q1; rz(param0) q1; cx q0,q1; h q1; } gate ecr q0,q1 { rzx(pi/4) q0,q1; x q0; rzx(-pi/4) q0,q1; }

Is this guidance on constructing this macro from their documentation? Or is this what is returned when using the built in Qiskit function (Here) for dumping a QuantumCircuit object to either OpenQASM 2 or 3?

I.e. if you construct a Qiskit circuit with an ECR gate, is this macro what is returned in the QASM by their own functions?

pcwysoc commented 5 months ago

@coreyostrove Using the Qiskit function to dump to QASM 2 from an ECR-containing circuit. There's no particular guidance available on how to interface QASM and ECR gates from IBM.

coreyostrove commented 5 months ago

Got it, one last question for you in that case (apologies for the inundation): What happens when you instead use QASM 3?

Based on what you've described, it sounds like the following sequence of operations would result in an error:

  1. Generate a Qiskit circuit containing an ECR gate directly (i.e. natively in Qiskit).
  2. Run that circuit using backend.run() and confirm whether it works.
  3. Dump that Qiskit circuit to QASM (with whether using QASM2 vs QASM3 matters still tbd)
  4. Read that QASM back into a Qiskit circuit using either the from_qasm_str or from_qasm_file constructors.
  5. Run that newly instantiated circuit using backend.run() again.

If we find that step 2 succeeds while step 5 fails then this indeed sounds like it is an IBM problem (they should be able to handle the round trip in their serialization/deserialization, though I am empathetic to how tricky that is sometimes).

As for how we would work around this, I'm not sure. If the use of macros is the problem, then maybe we could inline it?

pcwysoc commented 5 months ago

@coreyostrove I don't believe QASM 3 would fix the problem, but worth investigating.

I believe the sequence of operations would result in an error. I'll put running through that sequence for both QASM 2 and QASM 3 on my to-do list and get back to you in a couple days with the results and a test notebook.

The current thought for a workaround is to just convert pyGSTi circuits to Qiskit circuits without the QASM intermediary. The downsides are that that would take some time to implement and we would have to be very careful to avoid excessive transpilation as @jordanh6 pointed out above.

sserita commented 5 months ago

I'll also note that Qiskit 1.0 has QASM 3 (experimental) support. So an interface to Qiskit circuit directly could also give us QASM 3 support "for free", modulo all the issues that direct interface would have... I just bring it up since QASM 3 is something we've also been thinking about, and we'll probably only have time to implement QASM 3 or Qiskit and it would be nice if we got the other as a result.

pcwysoc commented 5 months ago

Hmm. That's an interesting point. I'm currently a little worried that Qiskit 1.0 might torpedo QASM 2.0 support inadvertently as the qasm2 convertor seems to live in the unfortunate qiskit-terra package.

From https://docs.quantum.ibm.com/api/qiskit/qasm2:

"The parser components of this module started off as a separate PyPI package: qiskit-qasm2(opens in a new tab). This package at version 0.5.3 was vendored into Qiskit Terra 0.24. Any subsequent changes between the two packages may not necessarily be kept in sync.

There is a newer version of the OpenQASM specification, version 3.0, which is described at https://openqasm.com(opens in a new tab). This includes far more facilities for high-level classical programming. Qiskit has some rudimentary support for OpenQASM 3 already; see qiskit.qasm3 for that.

OpenQASM 2 is not a suitable serialization language for Qiskit’s QuantumCircuit. This module is provided for interoperability purposes, not as a general serialization format. If that is what you need, consider using qiskit.qpy instead."

pcwysoc commented 5 months ago

@coreyostrove I ran:

  1. 1-2 in your list above for QASM 2
  2. 1-5 in your list above for QASM 2
  3. 1-2 in your list above for QASM 3
  4. 1-5 in your list above for QASM 3
  5. Modifying the convert_to_qasm method to add the macro/gate specification and then running an ECR-containing circuit through the usually pygsti.ibmq interface

All five of these circuits ran successfully. I believe something must have been patched since I last worked on this issue, since I remember running into issues on 2 and 5 (I did not test 3-4).

coreyostrove commented 5 months ago

Huzzah! While it sucks when stuff is broken, it is always nice when we aren't the ones who need to fix it. Since you said you were able to successfully modify the convert_to_qasm method using the specified macros and that it appears to be working through the ibmq interface I suggest we move forward with that as our tentative fix for this problem. @sserita has a branch under active development called feature-svb-qol-updates that includes a bunch of IBM-related updates, so this would probably fit in well there (we have an open PR #379 which we're looking at getting merged into develop within the next few weeks). Or you could spin up a new branch with this change (forked off of develop) and we can make a separate PR just for this. Either way should be fine.

pcwysoc commented 5 months ago

Any of those solutions work for me, just let me know what you and @sserita would like to do. It's about a four line fix. Add the macro in the convert_to_qasm function and then add the ECR gate in internal gates.

sserita commented 1 month ago

Completed with #457