Skip to content

Micro-optimizations to WaitGate #7671

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Sep 26, 2025
Merged

Micro-optimizations to WaitGate #7671

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
620e83a
Micro-optimizations to WaitGate
dstrain115 Sep 25, 2025
e5327b8
How embarassing! My commented code is showing!
dstrain115 Sep 25, 2025
fffc36a
sympy is basic
dstrain115 Sep 25, 2025
e768246
not more is parameterized.
dstrain115 Sep 26, 2025
ee34a74
Dont call twice.
dstrain115 Sep 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions cirq-core/cirq/ops/wait_gate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,10 @@ def __init__(
ValueError: If the `qid_shape` provided is empty or `num_qubits` contradicts
`qid_shape`.
"""
self._duration = value.Duration(duration)
if not protocols.is_parameterized(self.duration) and self.duration < 0:
self._duration = (
duration if isinstance(duration, value.Duration) else value.Duration(duration)
)
if not self.duration._is_parameterized_() and self.duration.total_picos() < 0:
Copy link
Collaborator

Choose a reason for hiding this comment

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

On my Debian box the time saving is ~300ns from 2.2 to 1.9µs.

If the overall time save of this PR is ~11µs is it worth to call a dunder method of different class rather than standard protocols.is_parameterized?

May be we could have a similar time save by reordering the checks in the is_parameterized call.

Copy link
Collaborator Author

@dstrain115 dstrain115 Sep 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

How about this? I changed it to not use is_parameterized at all. I just call total_picos and verify it's not a sympy expr before I verify it's greater than 0:

        total_picos = self.duration.total_picos()
        if not isinstance(total_picos, sympy.Basic) and total_picos < 0:
            raise ValueError('duration < 0')

From my calculation, this is even faster and avoids the private method call.

raise ValueError('duration < 0')
if qid_shape is None:
if num_qubits is None:
Expand Down
6 changes: 3 additions & 3 deletions cirq-core/cirq/value/duration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,19 +84,19 @@ def __init__(
"""
self._time_vals: list[_NUMERIC_INPUT_TYPE] = [0, 0, 0, 0]
self._multipliers = [1, 1000, 1000_000, 1000_000_000]
if value is not None and value != 0:
if value is not None:
if isinstance(value, datetime.timedelta):
# timedelta has microsecond resolution.
self._time_vals[2] = int(value / datetime.timedelta(microseconds=1))
elif isinstance(value, Duration):
self._time_vals = value._time_vals
else:
elif value != 0:
raise TypeError(f'Not a `cirq.DURATION_LIKE`: {repr(value)}.')
input_vals = [picos, nanos, micros, millis]
self._time_vals = _add_time_vals(self._time_vals, input_vals)

def _is_parameterized_(self) -> bool:
return protocols.is_parameterized(self._time_vals)
return any(isinstance(val, sympy.Expr) for val in self._time_vals)
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit - maybe sympy.Basic as below?

Cirq/cirq-core/cirq/protocols/resolve_parameters.py

Lines 79 to 80 in c8d8d59

if isinstance(val, sympy.Basic):
return True


def _parameter_names_(self) -> AbstractSet[str]:
return protocols.parameter_names(self._time_vals)
Expand Down