Enhancements and bugfixes in Report.write_notebook #658
Conversation
…rates. Make changes to get that test to pass.
…cceed when Estimate.circuit_weights is a nontrivial dict keyed by Circuit objects. This solution to enabling Estimate.write(...) is not the most elegant. It also doesnt make Estimate NicelySerializable. But this solution is useful.
…N serialization (as opposed to pickling, which was used up until this commit)
…ons. Use these to simplify code in write_notebook (both code executed there and code generated there). Add a test for Report.write_notebook which uses kwarg use_pickle=True.
There was a problem hiding this comment.
Glad to see the ErrgenTable fixed!
There was a problem hiding this comment.
I appreciate this warning.
…t removing the previous fix just for now.
| return self.IS_SIMPLE | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
There should only be two blank lines between class definitions.
| self._bare_init(labels, my_line_labels, editable, name, stringrep, | ||
| occurrence, compilable_layer_indices_tup) | ||
|
|
||
|
|
There was a problem hiding this comment.
There should only be one blank line between method definitions.
| self._hashable_tup = self.tup | ||
| self._hash = hash(self._hashable_tup) | ||
|
|
||
|
|
There was a problem hiding this comment.
Two blank lines between class definitions.
There was a problem hiding this comment.
Deleted several spurious blank lines
There was a problem hiding this comment.
Changes to this file fix a longstanding bug about calling .write() on Estimate objects whose circuit_weights member is a non-empty dict. The fix is to transform circuit_weights from a dict[Circuit,float] to a dict[str,float] before serializing.
pygsti/protocols/estimate.py
Outdated
| None | ||
| """ | ||
| if self.circuit_weights is not None: | ||
| self.circuit_weights = {getattr(c,'str',c): v for c,v in self.circuit_weights.items()} |
There was a problem hiding this comment.
The dict comprehension uses getattr in case self.circuit_weights is already keyed by strings.
rileyjmurray
changed the title
There was a problem hiding this comment.
Added newline at end of file
There was a problem hiding this comment.
Added newline at end of file
There was a problem hiding this comment.
nbformat is needed for tests in test_packages to run successfully. I can't figure out if nbformat is an explicit dependency of notebook (it is, after all, the reference implementation of the Jupyter notebook format), but there's no harm in making its dependency explicit for us.
There was a problem hiding this comment.
Looks great, thank you for your effort purging these pickle stragglers, @rileyjmurray!
3 participants
This resolves issue #655.
There's a change to the public API for Report.write_notebook and changes to the code in the notebook it generates. Report.write_notebook only saves data as a pickle if it's called with kwarg
use_pickle=True. There's a new Report.results_dict_from_dir function that can accept either the a filename with a.pklextension or a directory name. If the pkl extension is present then we try to load from a pickle (to ensure backward-compatibility). Otherwise, we try to load from a directory generated by the new Report.write_results_dict function.I introduced a little testing infrastructure so our write_notebook test actually runs the notebooks that it generates. I also added a test that that calls Report.write_notebook with kwarg
use_pickle=True.