Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BarrierOp and Circuit._add_conditional_barrier #988

Merged
merged 43 commits into from
Sep 6, 2023
Merged

Conversation

sjdilkes
Copy link
Contributor

Adds a new method to the Circuit object _add_conditional_barrier.
Previously OpType::Barrier was classified as a MetaOp. However, it seemed a bit at odds with other MetaOp, taking a variable number and mix of UnitID, and having special case handling for being added to a Circuit by a user, while other MetaOp can't be.
Given this, this new additional caveat of being to add them as a conditional gate seemed like a step far enough to make them a new child Op BarrierOp. Happy to undo this work and just have as a special case MetaOp if preferred though.

I have made the method "hidden" currently as it is unusual functionality - but obviously we can give it a normal name if we prefer.

@sjdilkes sjdilkes requested a review from cqc-alec August 29, 2023 08:56
Copy link
Collaborator

@cqc-alec cqc-alec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, code looks good. One suggestion I'd make, before merging this, would be to build a wheel and verify that it solves the problem this was designed to solve. Because it does feel like a strange feature -- "conditional barrier" doesn't really have semantic meaning at the circuit level (condition is evaluated at runtime; barrier is a compile-time annotation.)

pytket/pytket/qasm/qasm.py Show resolved Hide resolved
pytket/tests/qasm_test.py Outdated Show resolved Hide resolved
pytket/binders/circuit/Circuit/add_op.cpp Outdated Show resolved Hide resolved
pytket/binders/circuit/Circuit/add_op.cpp Outdated Show resolved Hide resolved
pytket/binders/circuit/Circuit/add_op.cpp Outdated Show resolved Hide resolved
tket/src/Circuit/basic_circ_manip.cpp Outdated Show resolved Hide resolved
tket/src/Ops/BarrierOp.cpp Outdated Show resolved Hide resolved
tket/src/Ops/BarrierOp.cpp Outdated Show resolved Hide resolved
tket/src/Ops/BarrierOp.cpp Outdated Show resolved Hide resolved
tket/test/src/Circuit/test_Circ.cpp Show resolved Hide resolved
@sjdilkes
Copy link
Contributor Author

sjdilkes commented Sep 4, 2023

Thanks, code looks good. One suggestion I'd make, before merging this, would be to build a wheel and verify that it solves the problem this was designed to solve. Because it does feel like a strange feature -- "conditional barrier" doesn't really have semantic meaning at the circuit level (condition is evaluated at runtime; barrier is a compile-time annotation.)

Good idea. I've messaged Dominic to check that the qasm output is valid for their input and messaged Kentaro to make sure that it suitably constructs his desired circuits.

@sjdilkes sjdilkes requested a review from cqc-alec September 5, 2023 15:00
@sjdilkes
Copy link
Contributor Author

sjdilkes commented Sep 6, 2023

Thanks, code looks good. One suggestion I'd make, before merging this, would be to build a wheel and verify that it solves the problem this was designed to solve. Because it does feel like a strange feature -- "conditional barrier" doesn't really have semantic meaning at the circuit level (condition is evaluated at runtime; barrier is a compile-time annotation.)

Good idea. I've messaged Dominic to check that the qasm output is valid for their input and messaged Kentaro to make sure that it suitably constructs his desired circuits.

Dominic has confirmed that the output qasm is as expected.

pytket/binders/circuit/Circuit/add_op.cpp Outdated Show resolved Hide resolved
pytket/pytket/qasm/qasm.py Outdated Show resolved Hide resolved
tket/include/tket/Ops/MetaOp.hpp Show resolved Hide resolved
@sjdilkes sjdilkes requested a review from cqc-alec September 6, 2023 10:26
@sjdilkes sjdilkes merged commit 99debd3 into develop Sep 6, 2023
33 checks passed
@sjdilkes sjdilkes deleted the conditional-barrier branch September 6, 2023 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants