search

sandialabs / pyGSTi

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

Method `parameter_labels()` returns an empty array #456

Open Pablovinas opened 2 weeks ago

Pablovinas commented 2 weeks ago

Describe the bug After adding operations to an explicit model constructed via pygsti.models.ExplicitOpModel(), the method parameter_labels() returns an empty array and does not recognize the model's parameters. If the model has been initially imported from pygsti.modelpacks, the array is not updated with the new parameters.

To Reproduce Steps to reproduce the behavior:

  1. Create a model, such as those defined in the the ExplicitModel.ipynb tutorial. We can additionally define custom operations as in the CustomOperator.ipynb tutorial and keep track of our parameters after naming them using the parameter_labels(self) property.
  2. Once the model is created, use mdl.parameter_labels to get the vector of model member's parameters within the model. It returns an empty array, causing further issues such as the collect_parameters()method not working.

Expected behavior I would expect the labels and parameters defined in the operations to be automatically recognised and updated in the model containing them.

Environment:

Additional context I have been able to circumvent this issue by explicitly using mdl._rebuild_paramvec(). However, I believe this should be done automatically when defining the model.

coreyostrove commented 21 hours ago

Hi @Pablovinas, thanks for the detailed report!

This isn't a bug per se, but rather related to a fairly old design choice to make the parameter management for models update lazily. As you've identified above, as currently implemented simply assigning a new value to a model member in one of the Model object's member dictionaries doesn't actually force a rebuild (i.e. a call to _rebuild_paramvec) in and of itself. I wasn't around for the original implementation of this, but I imagine it was for performance related considerations (rebuilding the parameter vector is actually a pretty expensive operation). The downside, as you discovered, is unexpected behavior wherein the values returned for certain model attributes can fall out of sync with the correct values depending on when you make the queries. This is not a particularly great behavior to have and it has tripped me up a number of times as well, so I think this is a good opportunity to fix this.

There are a couple options I can think of on how to proceed, so I wanted to open up some discussion on this to see what other folks think.

  1. Keep things lazy, but audit Model and add additional rebuild checks for publicly facing properties where we identify a potential for post-assignment inconsistencies.
  2. Modify the behavior of OrderedMemberDict, adding a call to _rebuild_paramvec to the __setitem__ method here (https://github.com/sandialabs/pyGSTi/blob/d27e4e688a64914a0b7aaec1de91aa56f7ce70c2/pygsti/models/memberdict.py#L352).
  3. Both 1 and 2.

I suspect the answer is 3, but am open to thoughts on this. Option 2 would address the particular problem that you encountered above, but it would leave the door open to other possible avenues for desynchronization. As an example, models typically inherit their parameter labels from the parameter labels of the constituent model member objects (though not always strictly so, as in the case of parameter collection which you mentioned using above). With just the fix in option 2 it would be possible to create a desynchronization condition by querying the model's parameter labels immediately after manually updating the parameter labels for one of it's children. Option 1 would address this, but has the potential for non-trivial performance impacts if we aren't careful, so I'd want to check to make sure we aren't calling the parameter_labels property in any particularly hot loops before committing to that change.