search

sandialabs / pyGSTi

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

Forward Simulator Dprobs Bugs with Instruments #453

Open pcwysoc opened 5 months ago

pcwysoc commented 5 months ago

Just posting this for tracking purposes and as notes for myself. While working on a modelmember that uses a new parameterization of a quantum instrument, I encountered two instances where the calculation dprobs did not work correctly. Both of these instances are due to the assumption the instrument elements are individually parameterized rather than inheriting from instrument itself. These two instances are:

  1. Calculation of the dprobs using a matrix forward simulator.
  2. Calculation of the dprobs using a map forward simulator and Cython.

The sequence of relevant functions in either of these cases is:

dprobs (forwardsim.py) --> bulk_fill_dprobs (forwardsim.py) --> _bulk_fill_dprobs (distforwardsim.py) --> _bulk_fill_dprobs_atom

This last function depends on the type of simulator (map or matrix) and will be found in the corresponding file (mapforwardsim.py and matrixforwardsim.py). One final level leads us to the problem:

  1. The matrixforwardsim.py function _bulk_fill_dprobs_atom references _compute_dproduct_cache (specifically around line 835) which computes the dprobs using a combination of the product rule and chain rule by circuit layer. It assumes that the instrument can be broken down into instrument elements each with their own independent parameterizations to do so. This is no longer true with the new parameterization.

  2. The mapforwardsim.py function _bulk_fill_dprobs_atom references calclib.mapfill_dprobs_atom (calclib is defined lines 86-96 in _set_evotype). The associated file is pygsti.forwardsims.mapforwardsim_calc_densitymx for Cython and the densitymx evotype. For Python, the associated file is pygsti.forwardsims.mapforwardsim_calc_generic. In the Cython file, the likely problem occurs around lines 326-328, where instrument elements seem to be referenced rather than the instrument itself.

TLDR: Currently, we have identified a workaround: use a map simulator and force mapforwardsim.py to use the Python implementation rather than the Cython implementation. However, these sections are necessary for distributed computing of GST and should be repaired prior to merge the new instrument parameterization.

Environment (please complete the following information):

pcwysoc commented 4 days ago

This may have been avoided when I created the ComposedInstrumentOp class. We should make a note to revisit this if we decide to move away from the Instrument/InstrumentOp framework.