Remove phase from CircuitData::with_capacity#15571
Conversation
Setting the global phase of either `CircuitData` or `DAGCirucit` to a floating-point value is infallible, but we didn't previously expose a way to do that. This is a small step on the way to making more of our Rust-native operations statically provably infallible.
This modifies the error handling in `ObjectRegistry` to make the error returns from the functions more explicit, and to add methods that allow short-hand assertions about the allowed failure modes. This is a stepping stone to making more circuit functions infallible when called from Rust space.
When we can statically state that the qubits we're adding are new anonymous qubits, the only way the qubit-adding functions can fail is by going over capacity. This commit exposes a way to do this safely, reducing one of the failure modes of `CircuitData::with_capacity`, which can provably not error during bit expansion because the maximums of the `num_qubits` and `num_clbits` arguments are within the capacity.
This makes the function infallible (as it _should_ be!). The only real failure mechanism of the function is by passing a global-phase parameter of an incorrect `Param` variant. When the global-phase is known to be either a symbol expression or a float, setting the global phase on an empty circuit is guaranteed to be infallible; setting the global phase to a float is _always_ infallible, while setting the global phase to a symbolic expression can only fail if there are duplicated symbol names. This cannot happen for empty circuits, since `ParameterExpression` enforces name uniqueness in the same way that `CircuitData` does.
jakelishman
added
type: qa
|
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 20995839947Details
💛 - Coveralls |
gadial
left a comment
gadial
left a comment
There was a problem hiding this comment.
Left some minor comments, overall seems good to me.
| #[pyo3(name="add_qubit", signature = (bit, *, strict=true))] | ||
| pub fn py_add_qubit(&mut self, bit: ShareableQubit, strict: bool) -> PyResult<()> { | ||
| Ok(self.add_qubit(bit, strict)?) | ||
| #[pyo3(signature = (bit, *, strict=true))] |
There was a problem hiding this comment.
While I don't object to this change, I'm interested as to why we are doing it - i.e. consolidating what before was a rust method + thin py wrapper into a single function (here and in other instances) - was there a reason for the wrapper approach?
There was a problem hiding this comment.
This change is actually in #15570, which in turn is built on #15564 - we might want to look at those separately first.
In short, though - I suspect Matt added the separation (it's very recent) not realising that you're allowed to return anything that implements Into<PyErr> in the error type, or he was thinking about a possible conditional compilation of the Python components of CircuitData. For exposing more things to C safely, though, and better control-flow sharing, we'll need more than just a separation of the pymethods and regular methods, so I'm not too worried.
I can separate them out again if you prefer.
|
|
||
| pub fn set_global_phase_f64(&mut self, phase: f64) { | ||
| self.clear_global_phase(); | ||
| self.global_phase = Param::Float(phase.rem_euclid(::std::f64::consts::TAU)); |
There was a problem hiding this comment.
Why are we doing this modulo? Is it possible that a user will be interested in setting a large global phase, losing this data as a result?
There was a problem hiding this comment.
Mostly because we've just always done this - this isn't a behaviour change.
| // Add an object to the registry, panicking if it is a duplicate or out of capacity. | ||
| pub fn add_unique_within_capacity(&mut self, object: B) -> T { | ||
| self.add(object) | ||
| .unwrap_or_else(|e| panic!("{}", e.to_string())) |
5 participants
Summary
This makes the function infallible (as it should be!). The only real failure mechanism of the function is by passing a global-phase parameter of an incorrect
Paramvariant. When the global-phase is known to be either a symbol expression or a float, setting the global phase on an empty circuit is guaranteed to be infallible; setting the global phase to a float is always infallible, while setting the global phase to a symbolic expression can only fail if there are duplicated symbol names. This cannot happen for empty circuits, sinceParameterExpressionenforces name uniqueness in the same way thatCircuitDatadoes.Details and comments
Built on #15570 and #15562. This is where I've been going with the infallibility - it was seriously annoying me that we were forcing awkward
unwrapor fallibility checks on basic circuit construction.