Add helper to detect nested control-flow writes to variables #10975
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds the representation of `expr.Var` nodes that own their own storage locations, and consequently are not backed by existing Qiskit objects (`Clbit` or `ClassicalRegister`). This is the base of the ability for Qiskit to represent manual classical-value storage in `QuantumCircuit`, and the base for how manual storage will be implemented.
This does not yet add the implementation of `QuantumCircuit.store`, which will come later as part of expanding the full API of `QuantumCircuit` to be able to support these runtime variables. The `is_lvalue` helper is added generally to the `classical.expr` module because it's generally useful, while `types.cast_kind` is moved from being a private method in `expr` to a public-API function so `Store` can use it. These now come with associated unit tests.
This adds all the new `QuantumCircuit` methods discussed in the variable-declaration RFC[^1], and threads the support for them through the methods that are called in turn, such as `QuantumCircuit.append`. It does yet not add support to methods such as `copy` or `compose`, which will be done in a follow-up. The APIs discussed in the RFC necessitated making `Var` nodes hashable. This is done in this commit, as it is logically connected. These nodes now have enforced immutability, which is technically a minor breaking change, but in practice required for the properties of such expressions to be tracked correctly through circuits. A helper attribute `Var.standalone` is added to unify the handling of whether a variable is an old-style existing-memory wrapper, or a new "proper" variable with its own memory. [^1]: Qiskit/RFCs#50
This adds an inner check to the control-flow operations that their blocks do not contain input variables, and to `QuantumCircuit.append` that any captures within blocks are validate (in the sense of the variables existing in the outer circuit). In order to avoid an `import` on every call to `QuantumCircuit.append` (especially since we're already eating the cost of an extra `isinstance` check), this reorganises the import structure of `qiskit.circuit.controlflow` to sit strictly _before_ `qiskit.circuit.quantumcircuit` in the import tree. Since those are key parts of the circuit data structure, that does make sense, although by their nature the structures are of course recursive at runtime.
This helper function is intended for use during the `DAGCircuit` construction in the near term, where it will need to distinguish whether a control-flow operation reads or writes a variable. This is not an efficient operation on the `QuantumCircuit` tree representation, and in general is something that should instead be calculated by data-flow analysis (or similar) on a graph-based representation of the program, or cached. However, `DAGCircuit` currently does not represent control-flow well enough to do anything like this across the circuit, so the helper method is necessary in the short term, despite the effective quadratic complexity on the control-flow depth of a circuit during complete recursive conversion of blocks to `DAGCircuit`s.
jakelishman
added
the
Changelog: New Feature
|
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 6409702336
💛 - Coveralls |
|
This will be rewritten in other forms. Closing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This helper function is intended for use during the
DAGCircuitconstruction in the near term, where it will need to distinguish whether
a control-flow operation reads or writes a variable.
This is not an efficient operation on the
QuantumCircuittreerepresentation, and in general is something that should instead be
calculated by data-flow analysis (or similar) on a graph-based
representation of the program, or cached. However,
DAGCircuitcurrently does not represent control-flow well enough to do anything
like this across the circuit, so the helper method is necessary in the
short term, despite the effective quadratic complexity on the
control-flow depth of a circuit during complete recursive conversion of
blocks to
DAGCircuits.Details and comments
Depends on #10974
Close #10940.
It might be preferable to store the number of writes to variables within
QuantumCircuit, but the only way to reliably keep this number up-to-date is to add an extra check withinQuantumCircuit._append, and I'm wary of affecting the performance of that inner-loop function for a feature that most circuits will not use.DAGCircuitis already very inefficient with regards to classical handling at the moment and needs a more complete rework, so this feels like a reasonable compromise; performance is poorer than it could be for control-flow ops inDAGCircuit(it's already poor), but circuits that don't use it aren't affected.