Refactor operator estimation and replace measure_observables #127
Conversation
…f outputs to pre-symmetrized program.
…handling identity.
…generate_programs.
forest/benchmarking/compilation.py
Outdated
| @@ -57,6 +57,17 @@ def _RY(angle, q): | |||
| return p | |||
|
|
|||
|
|
|||
| def _RX(angle, q): | |||
| """ | |||
| A RX in terms of RX(+-pi/2) and RZ(theta) | |||
There was a problem hiding this comment.
The docstring doesn't match the generated Program. I think you intended to swap RZ and _RY below?
… update test creation of experiments to prevent deprecation warnings.
kylegulshen
changed the title
There was a problem hiding this comment.
Looks good.
I agree with your suggestion of observable_estimation.py as this module does not estimate generic operators.
I have some minor comments, fix them if you think they improve things. I'll send you a slack with one larger comment that we might need to discuss in person. None of the above should block you from merging.
|
|
||
| @property | ||
| def in_operator(self): | ||
| warnings.warn("ExperimentSetting.in_operator is deprecated in favor of in_state", |
There was a problem hiding this comment.
again maybe something we can debate later.
Im in three minds [1] about in_state . We seem to have a conceptually mixed abstraction. We use an in_state and have an out observable often we conjugate Pauli inputs by Cliffords (we can conjugate by anything if we want but the DFE stuff works on the group property of cliffords). This seems like a Heisenberg representation of the circuit. But the in state is Schrodinger like.
One way forward would be to have an object prefix_program (with metadata saying if it is clifford or not) adn a sufix_program.
Anyway this issue is probably too large for this PR to take on, it would need input from others...
[1] Well two, but I dislike saying "I'm in two minds about X"
| if use_basic_compile: | ||
| programs.append(basic_compile(total_prog)) | ||
| else: | ||
| programs.append(total_prog) |
There was a problem hiding this comment.
again not a problem now, but for future use by others do we a compilation step in the else branch?
forest/benchmarking/compilation.py
Outdated
| def _RX(angle, q): | ||
| """ | ||
| A RX in terms of RZ(+-pi/2) and _RY(theta), which is itself decomposed | ||
| into native gates RX(+-pi/2) and RZ(theta) above. |
There was a problem hiding this comment.
Ok, you've made the docstring consistent with the program, but this way of compiling an RX doesn't make much sense to me, unless you are trying to re-use code from the definition of _RY. This is one case where that code re-use instinct works counter to code readability, and so you should instead just write a proper definition of RX in terms of Zs and RX(pi/2)s.
There was a problem hiding this comment.
I'm still not sure if I executed your suggestion correctly--if you mean that the sequence of gates itself should be different, then I'm not sure what sequence is simpler. The sequence here matches the output of the compiler.
|
wait, why are you copying all of this in from pyquil? Why not just use the pyquil version and contribute changes there, "upstream"? Or layer your changes on top of it |
|
Hey Matt, based on some requirements from the lower level of the stack we (well Kyle) are refactoring |
* Update DFE to match changes to operator estimation. * Fix op_est calibration coefficient bug * In op_est tests seed qc where possible, reduce number of repetitions, and increase tolerance
* Clean up utils pauli term creation and change name. * Update tomography after operator_estimation changes. * Refactor RB and unitarity to use ObservablesExperiment (#133) * Separate out optional explicit rb sequence generation path * Make calculation of rb variance account for covariance. * Updated rb notebooks and changed specification of depths. * Make rb fitting consistent with qbspec
* Move qbspec tests to fitting tests * Adjusted T2 experiments including a bug fix for T2echo. * standardize qbspec fitting/plotting around probability of 'measuring 1'.
* Refactor rpe generation to be more general.
This is the first step in refactoring this repo to use a standard workflow and move away from dataframes.
operator_estimation.pyoriginated in this repo but was copied over to pyquil and modified extensively. The version of the file in fb/master is outdated, and only the pyquil version is currently ever used within forest benchmarking. Commit 504f032 replaces the outdated version with the latest version from pyquil. Subsequently commit 31aa7aa reflects my changes to the pyquil version along with the updates to the tests. Looking at the diff between these commits will be more instructive, and in particular looking at the changes to the tests should provide a general sense of the change in usage. I've also included a notebook with outputs preserved for convenient inspection of usage.Note that some things were simply renamed rather than substantively changed. The main update is to decompose measure_observables into several steps, each of which works with lists of programs.