-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add __cirq_debug__
flag and conditionally disable qid validations in gates and operations
#6000
Conversation
…gates and operations
__cirq_debug__
flag and conditionally disable qid validations in gates and operations
cirq-core/cirq/ops/raw_types_test.py
Outdated
cirq.__cirq_debug__.set(False) | ||
op = cirq.H(q0, q1) | ||
assert op.qubits == (q0, q1) | ||
h_op.validate_args([q0, q1]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be nice to add a context manager for changing the __cirq_debug__
flag and making that the default way of using it. This ensures that state is restored even if an exception happens, which makes it much easier to know the state of the flag when looking at code.
@contextlib.contextmanager
def with_debug(value: bool) -> Iterator[None]:
token = __cirq_debug__.set(value)
try:
yield
finally:
token.reset()
Then can use that in these tests:
with cirq.with_debug(False):
op = cirq.H(q0, q1)
assert op.qubits == (q0, q1)
h_op.valudate_args([q0, q1])
I think we should encourage using a mechanism like this and discourage setting cirq.__cirq_debug__
manually (other than maybe for interactive use).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think adding the context manager is fine, but I'd still like to set the default value to __debug__
so that users have a way to disable validations in existing codebase without major modifications in the code.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a context manager, PTAL
semi-serious question: did you benchmark whether the new if statements measurably slow down existing code with debug turned on? I don't know how common running python with |
@mpharrigan I did some more measurements as per your suggestion and here are the findings. SetupI benchmark different versions of Cirq for the time it takes to construct def constuct_operations(q: List[cirq.Qid]) -> Iterator[cirq.Operation]:
for q1, q2 in zip(q[:-1], q[1:]):
yield cirq.X(q1)
yield cirq.Y(q2)
yield cirq.CNOT(q1, q2)
n = 100_000
q = cirq.LineQubit.range(n)
%snakeviz [*constuct_operations(q)] Cirq master without
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with one minor comment.
cirq-core/cirq/_compat.py
Outdated
@@ -15,6 +15,8 @@ | |||
"""Workarounds for compatibility issues between versions and libraries.""" | |||
import contextlib | |||
import dataclasses | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: remove blank line and sort imports since contextvars
is part of the stdlib.
…n gates and operations (quantumlib#6000) * Add __cirq_debug__ flag and conditionally disable qid validations in gates and operations * fix mypy errors * Fix typo * Address comments and add a context manager * Address nit
This is the first PR in a series of PRs to provide cirq users to conditionally disable validations in cirq to boost runtime performance. The proposal for adding a
__cirq_debug__
flag is presented in #5988This PR adds:
cirq.__debug__
flag, which is thread safe and implemented using contextvars. The default value is set to__debug__
global constant in python.Gate.validate_args()
,Operation.validate_args
andGate.on_each
on the value ofcirq.__debug__
flag.Benchmarks on my laptop show improvement of about ~20%.
This should also make the cirq.contrib.hacks.disable_validation obsolete.
cc @maffoo @zchen088 @dabacon @dstrain115