Avoid unintended calls to ``ComplementPOVMEffect.to_vector()``

rileyjmurray commented 5 months ago

Describe the bug A call to the unimplemented function ComplementPOVMEffect.to_vector() can be triggered as a side effect of inspecting variable values.

I'll concede that pyGSTi probably doesn't make promises about when it's safe to inspect certain variable values. That said, this behavior was very surprising to me during debugging, so I want to document it here even if we don't prioritize changing it. See below for more information.

To Reproduce

Prepare a script that fits a "Full TP" GST model to [simulated] data using the SimpleMapForwardSimulator class as the forward simulator.

Slightly tweak the following code, to express the list comprehension that defines ereps with a for loop: https://github.com/sandialabs/pyGSTi/blob/7411a00ec81e67f50932b705e46426fc2d4701d8/pygsti/forwardsims/mapforwardsim.py#L64-L66 I.e., replace the line that defines ereps with

    ereps = []
    for elabel in spc.full_effect_labels:
        effect = self.model.circuit_layer_operator(elabel, 'povm')
        erep = effect._rep
        ereps.append(erep)

Set a breakpoint at the line erep = effect._rep.
Run your script.

Resume execution after hitting the breakpoint set at step 3. After stepping over this line a few times (three times, IIRC), you'll get something like the following printed to console:

pyGSTi/pygsti/algorithms/core.py:685: in run_gst_fit
  opt_result = _do_runopt(objective, optimizer, printer)
pyGSTi/pygsti/algorithms/core.py:969: in _do_runopt
  opt_result = optimizer.run(objective, profiler, printer)
pyGSTi/pygsti/optimize/customlm.py:323: in run
  opt_x, converged, msg, mu, nu, norm_f, f, opt_jtj = custom_leastsq(
pyGSTi/pygsti/optimize/customlm.py:545: in custom_leastsq
  f = obj_fn(global_x)  # 'E'-type array
pyGSTi/pygsti/objectivefns/objectivefns.py:4777: in lsvec
  self.model.sim.bulk_fill_probs(self.probs, self.layout)  # syncs shared mem
pyGSTi/pygsti/forwardsims/forwardsim.py:555: in bulk_fill_probs
  return self._bulk_fill_probs(array_to_fill, layout)
pyGSTi/pygsti/forwardsims/forwardsim.py:558: in _bulk_fill_probs
  return self._bulk_fill_probs_block(array_to_fill, layout)
pyGSTi/pygsti/forwardsims/forwardsim.py:562: in _bulk_fill_probs_block
  self._compute_circuit_outcome_probabilities(array_to_fill[element_indices], circuit,
pyGSTi/pygsti/forwardsims/mapforwardsim.py:59: in _compute_circuit_outcome_probabilities
  rhorep = self.model.circuit_layer_operator(spc.circuit_without_povm[0], 'prep')._rep
pyGSTi/pygsti/models/model.py:1479: in circuit_layer_operator
  self._clean_paramvec()
pyGSTi/pygsti/models/model.py:679: in _clean_paramvec
  clean_obj(obj, lbl)
pyGSTi/pygsti/models/model.py:675: in clean_obj
  clean_obj(subm, _Label(lbl.name + ":%d" % i, lbl.sslbls))
pyGSTi/pygsti/models/model.py:676: in clean_obj
  clean_single_obj(obj, lbl)
pyGSTi/pygsti/models/model.py:666: in clean_single_obj
  w = obj.to_vector()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <pygsti.modelmembers.povms.complementeffect.ComplementPOVMEffect object at 0x2b5db51b0>

  def to_vector(self):
      """
      Get the POVM effect vector parameters as an array of values.

      Returns
      -------
      numpy array
          The parameters as a 1D array with length num_params().
      """
>       raise ValueError(("ComplementPOVMEffect.to_vector() should never be called"
                        " - use TPPOVM.to_vector() instead"))
E       ValueError: ComplementPOVMEffect.to_vector() should never be called - use TPPOVM.to_vector() instead

Additional context

The error message says to run TPPOVM.to_vector() instead. I'll note that TPPOVM doesn't provide its own implementation of to_vector (at time of writing), but it inherits a seemingly-suitable implementation from BasePOVM.

I think my main priority here is understanding why this happens. There has to be something going on where accessing a field of effect is setting its parent model.dirty field to True. I'd like to know where this is happening and why it needs to happen.

It might be a valid fix to just have ComplementPOVMEffect.to_vector() return a numpy ndarray of shape (0,). That would work harmoniously with the implementation of ComplementPOVMEffect.from_vector.

sserita commented 3 months ago

Just updating with some discussion on an internal thread. This issue is due to an underlying problem with the dense interface code for states and effect model members, and will be fixed as a side effect for revamping/deprecating that interface.

rileyjmurray commented 2 months ago

Update. For some reason this bug is getting triggered on my branch for PyTorch-backed forward simulation:

rileyjmurray commented 1 month ago

The bug is now also triggered in a continuous integration build:

https://github.com/sandialabs/pyGSTi/actions/runs/9191386508/job/25277676157#step:8:725

sandialabs / pyGSTi

Avoid unintended calls to ``ComplementPOVMEffect.to_vector()`` #397