IBMQExperiment and FreeformDesign/CombinedExperimentDesign quality-of-life updates

[sserita]

This PR makes several quality-of-life improvements for IBMQExperiment, including checkpointing for IBMQ experiments mentioned in #327, as well as FreeformDesign and CombinedExperimentDesign serialization.

IBMQExperiment updates:

1. The IBMQExperiment class is changed from a dict to a full class that inherits from TreeNode, making it commensurate with serialization of objects such as ExperimentDesign and ProtocolData.
2. With a more granular serialization strategy, it is now possible to checkpoint IBMQExperiment objects. The API follows GST checkpointing where checkpoint_path and disable_checkpointing can be passed to the constructor (checkpointing is on by default). If checkpoint_path is provided, then transpile(), submit(), or retrieve_results will update the on-disk ibmqexperiment when possible.
3. If bson is installed, we can use the json_util hook to handle objects such as datetime.datetime which are present in submit_time_calibration_data and batch_results from the IBMQ server. Depending on the availability of bson, we can serialize to all text/json files, but fall back to pickle still.

FreeformDesign updates:

1. The aux_info member of FreeformDesign is written as JSON instead of pickle. To do this, first we cast the Circuit keys as strings and then write the now-JSONable dictionary to file. We convert back to Circuits on deserialization.
2. More controversially, I skip serializing all_circuits_needing_data for FreeformDesign. This field should match the keys of aux_info, and with the size of these designs, it saves quite a lot of space to not double save them (almost 1 GB for some of my SVB use cases!).

CombinedExperimentDesign updates:

1. The constructor now takes an additional skip_writing_all_circuits flag. The intention is that in cases where all_circuits_needing_data is simply the union of subdesigns, this can be regenerated on the fly. This saves space on disk and cuts the write/read times for the largest thing being written (cutting my 1 GB down to ~500 MB for my SVB use cases!).

I've done my best to make it backwards-compatible; for example, we can still load old pickle-style IBMQExperiment directories into the new object. However, the IBMQExperiment workflow has to change a little to enable checkpointing - a minor pain point that I hope is well worth it for most users.

Remaining Tasks

After a round of feedback, here's the current list of additional tasks:

• Check that instruments work with this
• Make checkpointing default and in-line with new GST checkpointing kwarg, if possible
• Add unit tests using Qiskit mock server
• Use new Qiskit Sessions to future-proof against eventual IBM Provider deprecation

[rileyjmurray]

@sserita, I got a ping about reviewing this due to being a code owner. From what I can see this doesn't touch code that I have prior experience with. Maybe a change in my team in the CODEOWNERS file is in order?

[coreyostrove]

Since we're enabling checkpointing by default I think it would be worthwhile implementing a default path here for when the checkpoint_path kwarg isn't set by a user. This something we currently do in the GST protocol checkpointing code, fwiw, in case referencing how it is handled in that part of the code would be useful.

[sserita]

I guess that is fair. One reason I did it this way is that the "checkpoint" here is actually a completely valid IBMQExperiment object that can be loaded using from_dir(), so in my experience so far it has been nice to have it match the dirname for write() so that the user doesn't have to change how they load from checkpoint vs a .write()... but it's a good point that a default path could simplify this for users. I'll think about how I want to incorporate that here.

[coreyostrove]

I think the general idea of having the option to skip writing all_circuits is alright, but having this be something that is specified as an attribute of a class instance during the constructor stage feels suboptimal. This feels like something that would fit better as an optional flag passed into the write method. Of course, doing so would mean you'd need to implement an overriding version of write specific to CombinedExperimentDesign. Then again it looks like this is the tactic that was taken for the FreeformDesign, so maybe that isn't so bad?

[sserita]

I actually tried that first, but it got a little nasty because a) it was doing a lot of manipulation of auxfile_types on the fly, and b) I really want to trigger that option as part of ProtocolData, IBMQExperiment, etc and that would mean plumbing up an option that was only used for CombinedExperimentDesign through everything that could possibly hold an edesign... Any suggestions on avoiding that plumbing are welcome, the class member was my first pass at it but I agree it's pretty awkward.

[sserita]

Actually I'm not sure why I didn't think of this, but I can just check whether all_circuits_needing_data matches the set of all subdesigns... Duh. (Idea came from your assert comment below.)

[coreyostrove]

Great work, @sserita! I posted a couple of code specific comments above, but had a couple of broader comments to go along with them. I think the main thing that this PR is missing is some unit tests. As far as I can tell we currently have no coverage of the IBMQExperiment in any of our test modules. More and more users are reliant on this code so this would be a good opportunity to at least start to fill that gap by adding some tests for the new checkpointing functionality. Thoughts on this? We obviously don't want to have the runners making a bunch of API calls to IBM's servers as part of testing, but I took a look at the code and at first glance it looks like the transpile method doesn't make any external API calls, so this could be a good candidate for testing. I forget off-hand whether using the simulator backend runs the simulations locally or on their cloud, but if it is locally that might be a good option for extending coverage to things like the submit method.

Relatedly, while we have some coverage on serialization for experiment designs, it looks like presently we don't have have anything for FreeformDesign (we actually only have one test that touched FreeformDesign period, and that is related to a qubit label mapping method), so this could be a good thing to add. Ditto with the new CombinedExperimentDesign serialization option (for skipping all_circuits).

[coreyostrove]

Would it be possible to add an assertion or other check here that check this is true? That is, that self.aux_info.keys() is equivalent to all_circuits_needing_data? This is definitely true initially upon construction, but it also looks like it is possible for these to become out of sync during the course of standard usage. For example, the base ExperimentDesign class has the method truncate_to_circuits which is a wrapper around _truncate_to_circuits_inplace. Looking at the implementation of _truncate_to_circuits_inplace this operates directly on all_circuits_needing_data and we never re-sync this with aux_info. There are a couple other methods where this happens as well.

[sserita]

Good point. I think probably other things would break if they got out of sync too, but an assert here to check is very easy.

[coreyostrove]

First time using the reviewer interface, so didn't realize my other comments probably should have gone in here, sorry!
See above comments for more.

[pcwysoc]

@sserita This is non-urgent, please ignore until not OOO. I'm running into a bug when attempting to transpile edesigns with qubit labels that do not start at qubit 0 (i.e., qubit_labels = ('Q27', ) ):

QASM2ParseError Traceback (most recent call last)
Cell In[13], line 1
----> 1 exp1Q.transpile(backend)

File ~/pyGSTi/pygsti/extras/ibmq/ibmqexperiment.py:567, in IBMQExperiment.transpile(self, ibmq_backend, qiskit_pass_kwargs, qasm_convert_kwargs, num_workers)
563 # Run in parallel (p.imap) with progress bars (tqdm)
564 #with _mp.Pool(num_workers) as p:
565 # isa_circuits = list(_tqdm.tqdm(p.imap(task_fn, tasks), total=len(tasks)))
566 for task in _tqdm.tqdm(tasks):
--> 567 self.qiskit_isa_circuit_batches.append(task_fn(task))
569 # Save single batch
570 chkpt_path = _pathlib.Path(self.checkpoint_path) / "ibmqexperiment"

File ~/pyGSTi/pygsti/extras/ibmq/ibmqexperiment.py:63, in _transpile_batch(circs, pass_manager, qasm_convert_kwargs)
60 for circ in circs:
61 # TODO: Replace this with direct to qiskit
62 pygsti_openqasm_circ = circ.convert_to_openqasm(**qasm_convert_kwargs)
---> 63 qiskit_qc = _qiskit.QuantumCircuit.from_qasm_str(pygsti_openqasm_circ)
64 batch.append(qiskit_qc)
66 # Run pass manager on batch

File /opt/homebrew/Caskroom/miniconda/base/envs/pygsti_1_new_qiskit/lib/python3.10/site-packages/qiskit/circuit/quantumcircuit.py:4025, in QuantumCircuit.from_qasm_str(qasm_str)
4022 # pylint: disable=cyclic-import
4023 from qiskit import qasm2
-> 4025 return qasm2.loads(
4026 qasm_str,
4027 include_path=qasm2.LEGACY_INCLUDE_PATH,
4028 custom_instructions=qasm2.LEGACY_CUSTOM_INSTRUCTIONS,
4029 custom_classical=qasm2.LEGACY_CUSTOM_CLASSICAL,
4030 strict=False,
4031 )

File /opt/homebrew/Caskroom/miniconda/base/envs/pygsti_1_new_qiskit/lib/python3.10/site-packages/qiskit/qasm2/init.py:587, in loads(string, include_path, custom_instructions, custom_classical, strict)
571 """Parse an OpenQASM 2 program from a string into a :class:.QuantumCircuit.
572
573 Args:
(...)
584 A circuit object representing the same OpenQASM 2 program.
585 """
586 custom_instructions = list(custom_instructions)
--> 587 return _parse.from_bytecode(
588 _qasm2.bytecode_from_string(
589 string,
590 [_normalize_path(path) for path in include_path],
591 [
592 _qasm2.CustomInstruction(x.name, x.num_params, x.num_qubits, x.builtin)
593 for x in custom_instructions
594 ],
595 tuple(custom_classical),
596 strict,
597 ),
598 custom_instructions,
599 )

File /opt/homebrew/Caskroom/miniconda/base/envs/pygsti_1_new_qiskit/lib/python3.10/site-packages/qiskit/qasm2/parse.py:211, in from_bytecode(bytecode, custom_instructions)
208 # Pull this out as an explicit iterator so we can manually advance the loop in DeclareGate
209 # contexts easily.
210 bc = iter(bytecode)
--> 211 for op in bc:
212 # We have to check op.opcode so many times, it's worth pulling out the extra attribute
213 # access. We should check the opcodes in order of their likelihood to be in the OQ2 program
214 # for speed. Gate applications are by far the most common for long programs. This function
215 # is deliberately long and does not use hashmaps or function lookups for speed in
216 # Python-space.
217 opcode = op.opcode
218 # OpCode is an enum in Rust, but its instances don't have the same singleton property as
219 # Python enum.Enum objects.

QASM2ParseError: ":7,5: index 27 is out-of-range for register 'q' of size 1"

[pcwysoc]

@sserita Also, I wanted to check to make sure the bug with using add_count_dict to populate the dataset was fixed (we had been using add_count_list as a work around). This is non-urgent, please ignore until no longer OOO.