BooleanHamiltonianGate implementation #4705
Conversation
There was a problem hiding this comment.
Should the BooleanHamiltonianGate be a CircuitOperation instead? Follows from the proposal in #4338 to replace composite gates with CircuitOperations.
@tanujkhattar Hmm, possibly? I think BooleanHamiltonian would be a denser serialization though, if that's a concern. @95-martin-orion should weigh in on that. |
|
I agree with @daxfohl on this one - the gate is preferable. I've gone back and forth a bit on this (as is evident from my comment history), but the point about denser serialization / easier comparison of composite gates vs. |
|
@tanujkhattar These kinda of "more compound style" gates worry me a little bit when it comes to playing nice with gatesets. Do you think there is a nice way to fit this gate and the one proposed in #4696 ? I'm less worried about the GlobalPhaseGate |
|
@MichaelBroughton Do you mean, remove both HamiltonianBooleanOperation and HamiltonianBooleanGate, and replace those with a module-level function I can see how that makes sense, since the benefit of compact serialization is only useful if the target device understands it. Currently nothing else understands what a BooleanHamiltonian is and would just barf. The downside is it no longer looks as nice in diagrams, just a plain subcircuit. Also a little less flexible in terms of I still feel like having the gate is useful for things like the latter. You don't get that flexibility if it's just an anonymous subcircuit. |
|
For a little bit of context, in my original attempt: I did have a standalone function rather than a dedicated gate. I am OK with going back to gate and I am just giving this information for historical context. Maybe Balint's reasons for going to a gate are important. If we do decide to go back to a function, I am OK with either Dax doing it, or I can take it up myself (in which case, please let me know where the code should reside). |
|
I'm obviously biased, but I'm 99% in favor of keeping the approach in this PR, and simply changing this from an operation to a gate. Gates are more flexible and designed for plug and play, hence the original issue to do this #4683. My second choice would be to change it to a factory method that creates a CircuitOperation, like Tony's original attempt, and remove BooleanHamiltonian as a class entirely. I'm not in favor of having this "be" a CircuitOperation via inheritance. CircuitOperation just brings a lot of cruft to sit on top of. Like it's odd that BooleanHamiltonian.with_measurement_key_mapping would exist, etc. I can see the perspective that "it's just like the factory method but better because now you can still do isinstance(BooleanHamiltonian)", but something still feels off to me with this approach. (If we really want a CircuitOperation that you can determine was a BoolHam, I'd offer that tags might be a better way to do this. But like I said, I think we should just use a gate) I also don't think we should leave it as-is. We've decided to move ops to gates where possible, and this is a pretty easy one, so we should do something here. |
|
@tanujkhattar Just checking in whether you're waiting on something from my side. (No rush, I can see you're busy with other things, just wanted to make sure I'm not missing something.) |
* BooleanHamiltonianGate implementation * Replace dict with sequence for order preservation * Remove logic for qudits, add check that only qubits are accepted. * Deprecate boolham op
* BooleanHamiltonianGate implementation * Replace dict with sequence for order preservation * Remove logic for qudits, add check that only qubits are accepted. * Deprecate boolham op
* BooleanHamiltonianGate implementation * Replace dict with sequence for order preservation * Remove logic for qudits, add check that only qubits are accepted. * Deprecate boolham op
Implementation for BooleanHamiltonianGate.
As implemented, BooleanHamiltonianGate.on(qubits) returns a GateOperation. And BooleanHamiltonianOperation has not been changed to contain any gate. In other words, BooleanHamiltonianGate is completely unrelated to BooleanHamiltonianOperation. They can be considered two different ways to do the same thing. This means we can simply deprecate BooleanHamiltonianOperation and have users replace it with BooleanHamiltonianGate.
The code for BooleanHamiltonianGate is basically identical with BooleanHamiltonianOperation, except instead of Dict[str, Qid]], a mapping is a Sequence[Tuple[str, int]], where the int represents the dimension. It implements the qid_shape protocol from this data, and the GateOperation constructor already has logic to ensure the appropriate number/dimension of qubits are applied.
Tests are added to all effects.
cc @tanujkhattar @tonybruguier