sandialabs / pyGSTi

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

ProtocolResults Incompletely Loads From Directory #382

Closed pcwysoc closed 3 months ago

pcwysoc commented 7 months ago

Describe the bug ProtocolResults objects written to directory do not have a 'dataset' attribute and the circuit_lists['final'] and circuit_lists['iteration'] both return NoneType. This breaks report generation. It is possible to work around it by using the underlying ProtocolData, i.e. gst_results.data.dataset and gst_results.data.edesign.circuit_lists.

I also encountered a bug in lines 174-175 of TPInstrument that MT_member has no attribute .submembers(). I was able to fix this using the following:

MT_member = next(filter(lambda pair: pair[1].index == len(lbl_member_pairs) - 1, lbl_member_pairs)) param_ops = MT_member.submembers() if type(MT_member) is tuple: param_ops = [] for lbl in lbl_member_pairs: param_ops += [lbl[1]] else: param_ops = MT_member.submembers() # the final (TP) member has all the param_ops as its submembers

To Reproduce Save a ProtocolResults object to directory, load from directory, and try to perform GST. To reproduce the second bug, create an instrument-containing edesign and perform the same steps.

Expected behavior The ProtocolResults object should load with the correct attributes, removing the need for workarounds.

Environment (please complete the following information):

Additional context Add any other context about the problem here.

coreyostrove commented 3 months ago

Hi @pcwysoc, thanks for taking the time to report this. I am unfortunately finding myself unable to replicate these particular bugs. For what it is worth, it seems that in the course of trying to reproduce this error I stumbled onto what appears to be the cause of the issue #397, so that is something. Is this still an issue you're encountering? If so, do you happen to have a script that reproduces this behavior (I realize this request coming months later means this may well not be the case).

pcwysoc commented 3 months ago

Hi @coreyostrove, I'll see if I can get you a minimum working example in the next few days. I'm glad it at least led to another open problem!

pcwysoc commented 3 months ago

@coreyostrove I wasn't able to replicate it either. It's possible something was fixed in between now and when I reported this. I tried it with both a QI-containing and a regular ProtocolResults.

coreyostrove commented 3 months ago

Excellent, not sure what if anything we did that would've fixed this, but I will not look a gift horse in the mouth. I'm going to mark this as closed, but if anything arises related to this issue feel free to re-open it.