sserita
commented
1 day ago OK I can confirm the second error message is a densitymx_slow evotype bug. The bug has to do with the case where a circuit is implicitly using a MarginalizedPOVM, i.e. it is using measuring a subset of the qubits defined in the model. This is a rarely used edge case, so not surprised it slipped under the radar until now.
Basically the difference is between the Cython directly computing effect reps: https://github.com/sandialabs/pyGSTi/blob/916877cba445b32219b2f4478da5375b05ffb37e/pygsti/forwardsims/mapforwardsim_calc_densitymx.pyx#L327 vs the Python computing for the general POVM effect first. https://github.com/sandialabs/pyGSTi/blob/916877cba445b32219b2f4478da5375b05ffb37e/pygsti/forwardsims/mapforwardsim_calc_generic.py#L45-L50
I'll use test_simulate from test/unit/objects/test_circuit.py to illustrate the issue.
https://github.com/sandialabs/pyGSTi/blob/916877cba445b32219b2f4478da5375b05ffb37e/test/unit/objects/test_circuit.py#L614-L627
This test uses a circuit defined on two qubits, but the model is defined on 4 qubits, so this is an implicit marginalized POVM.
The test constructs a LocalNoiseModel, so this is the relevant POVM lookup function:
https://github.com/sandialabs/pyGSTi/blob/916877cba445b32219b2f4478da5375b05ffb37e/pygsti/models/localnoisemodel.py#L495-L530
Both will fail the layerlbl in povm_blks check, but the else assumes that layerlbl is an effect label (e.g. "Mdefault_00"). This is true in the Cython (evotype "densitymx") case, but not in the Python (evotype "densitymx_slow") case. In fact, we are failing at the povmreps construction line just before the effect labels would be passed in.
Two possible solutions:
- Wrap the
povmrepsconstruction in a try/except that then moves onto the effect labels which should work (since this is the codepath "densitymx" is using). - Fix the LocalNoiseModel layer rules such that the POVM label is handled properly.
Solution 1 has the advantage that it is model/layer rule independent, but I kind of like Solution 2 more. I personally prefer not having try/excepts unless there really is no reasonable alternative. We will just have to make sure that every layer rule handles both the POVM and effect label cases. Thoughts?
As part of PR #452 we noticed that several "slow" evotype objects had an implementation for
__reduce__that was commented out. Some of these commented out definitions indicated a need to be fixed. That raised the question of if the existing slow evotype code is broken in some way. To start the discussion on this point I ran our unit tests with code coverage enabled. Zooming in on results for evotypes, I get the followingNote: I generated this report by running
inside the test directory.
Tests results when
Evotype.default_evotype = "densitymx_slow"The default evotype is densitymx as long as the cython extensions correctly compiled. Since we're trying to understand possible bugs in the slow evotypes I re-ran with densitymx_slow as the default evotype. This resulted in four failed unit tests.
Here's a comparison of coverage when using the densitymx_slow by default (top window) vs the plain densitymx evotype (bottom window):
Click here for details on test failures
``` ====================================================================================== ERRORS ======================================================================================= _________________________________________________________ ERROR at setup of InterpygateGSTTester.test_circuit_probabilities _________________________________________________________ [gw9] darwin -- Python 3.10.13 /Users/rjmurr/miniconda3/envs/pg310/bin/python cls =