New Feature: GST iteration-specific optimizers #690
Conversation
I maintain the https://github.com/conda-forge/pygsti-feedstock/ build for conda-forge. I noticed that `tqdm` is not required for pygsti. Since 57c7853 was merged.
Add `tqdm` as a proper dependency
Revert "Add `tqdm` as a proper dependency"
I also noticed that some gst unit tests were not being run. This was fixed by changing their name, and it revealed two failing tests which I believe are unrelated to my changes
juangmendoza19
requested review from
coreyostrove
and removed request for
a team
To clarify, I specifically said you'd get "street cred with @rileyjmurray, exchangeable at a future date for help with hard linear algebra problems". Whether the general public thinks of you as cooler now is still to be determined. |
Much better than any other type of street cred I can think of. |
test_packages/temp_test_files/test_GateSetTomography_serialization/badfit_options.json
Outdated
Show resolved
Hide resolved
juangmendoza19
changed the title
| if simulator is not None: | ||
| mdl_start.sim = simulator | ||
|
|
||
| if optimizers is None: |
There was a problem hiding this comment.
This logic is repeated from the algorithms/core.iterative_gst_generator function. I think we should refactor to call the same utility helper function in both cases. That way if we update the allowed types to pass in we will only need to update the logic in one location.
There was a problem hiding this comment.
I'll take this as an opportunity to soap box against DRY in this particular instance, and instead advocate for LoB (https://htmx.org/essays/locality-of-behaviour/).
| assert _np.allclose(mdl_result2.to_vector() , mdl_result1.to_vector()) | ||
| assert _np.allclose(mdl_result3.to_vector() , mdl_result1.to_vector()) | ||
| assert _np.allclose(mdl_result4.to_vector() , mdl_result1.to_vector()) | ||
|
|
There was a problem hiding this comment.
Did you also want to test if we can have different optimizer settings for each iteration of the GST experiment?
There was a problem hiding this comment.
I can add a test to check if it runs, but I don't see an easy way to test its valid. At that point it might not have any more value than the
results3 = proto1.run(self.gst_data, optimizers=optimizers)
test.
|
Quick comment: Github auto selected me as a reviewer, but since I helped write this code it wouldn't make sense for me to do so, in the off chance anyone was waiting for one from me. |
Corey and I pair-programmed a new feature (in hopes it will be useful for AMS) that allows for the specification of iteration-specific optimizers.
This new feature allows the user to pass in many optimizers into GST, such that each GST iteration can use a different optimizer.
For example, if a user wants to have a different number of iterations only for the final logl GST iteration, it can specify a different optimizer just for the final iteration.
To use this feature, simply create a list of the optimizers to be used:
Where k is the total number of iterations for the GST protocol (k == len(circuit_lists)).
Iteration i within GST will use optimizer_i
On a separate note, I found two GST unit test classes that were not being executed: TestStandardGST and TestGateSetTomography. Corey suggested their names must end with "Tester", so I renamed them "GateSetTomographyTester" and "StandardGSTTester".
This revealed two failing tests:
FAILED protocols/test_gst.py::GateSetTomographyTester::test_run_custom_sim - assert False
FAILED protocols/test_gst.py::StandardGSTTester::test_run_custom_sim - assert False
These tests are failing due to code that is not touched in this branch, and also fail in develop if named appropriately such that they are not skipped.
Also Corey promised I would get "street cred" for doing type annotations of any arguments touched in this PR. I hope this promise is fulfilled.