Add {Frozen}Circuit.from_moments to construct circuit by moments.
#5805
Conversation
In our internal code almost all circuits are constructed with explicit moments rather than using an insertion strategy because we want to control the alignment of ops. Here we add a `from_moments` classmethod on `Circuit` and `FrozenCircuit` which takes any number of `OP_TREE` args and converts each one to a `Moment` in the resulting circuit. This gives a convenient way to construct circuits without cluttering the code with multiple calls to `Moment`.
cirq-core/cirq/circuits/circuit.py
Outdated
| if len(contents) == 1 and isinstance(contents[0], AbstractCircuit): | ||
| self._moments.extend(contents[0].moments) |
There was a problem hiding this comment.
This could produce two circuits with shared Moments.
Are Moment immutable? If so this should be OK...
There was a problem hiding this comment.
Yes, moments are immutable.
cirq-core/cirq/circuits/circuit.py
Outdated
| Args: | ||
| *moments: Op tree for each moment. | ||
| """ | ||
| return cls(Moment(moment) for moment in moments) |
There was a problem hiding this comment.
Should it be cls(*(Moment(moment) for moment in moments))?
Right now it is taking the old branch on lines 1722-23.
|
On another look, the Circuit and FrozenCircuit constructors work as desired in their initial shape when passed arguments that are all Moment-s. I think we can do this by adding a single These can be pulled in with |
There was a problem hiding this comment.
I propose to do a shared implementation in AbstractCircuit.
See #5805 (comment).
|
I'd like the avoid the constructor overhead associated with using the insertion strategies in these cases. For example, even with #5332 to optimize circuit construction with the |
The new function _from_moments replaces the previous _with_sliced_moments
|
@pavoljuhas, thanks for the review. I've reverted the constructor changes, and instead implemented this in terms of a |
maffoo
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
the
automerge
CirqBot
removed
the
front_of_queue_automerge
…uantumlib#5805) In our internal code almost all circuits are constructed with explicit moments rather than using an insertion strategy because we want to control the alignment of ops. Here we add a `from_moments` classmethod on `Circuit` and `FrozenCircuit` which takes any number of `OP_TREE` args and converts each one to a `Moment` in the resulting circuit. This gives a convenient way to construct circuits without cluttering the code with multiple calls to `Moment`. Also adds special cases in both the `Circuit` and `FrozenCircuit` constructors to handle constructing from a single circuit or a sequence of moments.
…uantumlib#5805) In our internal code almost all circuits are constructed with explicit moments rather than using an insertion strategy because we want to control the alignment of ops. Here we add a `from_moments` classmethod on `Circuit` and `FrozenCircuit` which takes any number of `OP_TREE` args and converts each one to a `Moment` in the resulting circuit. This gives a convenient way to construct circuits without cluttering the code with multiple calls to `Moment`. Also adds special cases in both the `Circuit` and `FrozenCircuit` constructors to handle constructing from a single circuit or a sequence of moments.
In our internal code almost all circuits are constructed with explicit moments rather than using an insertion strategy because we want to control the alignment of ops. Here we add a
from_momentsclassmethod onCircuitandFrozenCircuitwhich takes any number ofOP_TREEargs and converts each one to aMomentin the resulting circuit. This gives a convenient way to construct circuits without cluttering the code with multiple calls toMoment.Also adds special cases in both the
CircuitandFrozenCircuitconstructors to handle constructing from a single circuit or a sequence of moments.