Micro-optimizations to WaitGate #7671
Conversation
- Noticed some inefficiencies in the creation of WaitGate where it is doing unnecessary object boxing. - This removes (slow) comparisons between Duration objects. This improves creation of WaitGate objects (e.g. from cirq.wait(nanos=10)) from about 27 micros to about 16 micros. While not a huge effect, this could have measurable savings for certain T1 and similar experiments that involve creation of lots of wait gates.
cirq-core/cirq/ops/wait_gate.py
Outdated
| duration if isinstance(duration, value.Duration) else value.Duration(duration) | ||
| ) | ||
| if not self.duration._is_parameterized_() and self.duration.total_picos() < 0: | ||
| # if not protocols.is_parameterized(self.duration) and self.duration.total_picos() < 0: |
There was a problem hiding this comment.
nit - remove comment-disabled code
cirq-core/cirq/value/duration.py
Outdated
|
|
||
| def _is_parameterized_(self) -> bool: | ||
| return protocols.is_parameterized(self._time_vals) | ||
| return any(isinstance(val, sympy.Expr) for val in self._time_vals) |
There was a problem hiding this comment.
nit - maybe sympy.Basic as below?
Cirq/cirq-core/cirq/protocols/resolve_parameters.py
Lines 79 to 80 in c8d8d59
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7671 +/- ##
==========================================
- Coverage 99.37% 99.36% -0.01%
==========================================
Files 1082 1082
Lines 96708 96711 +3
==========================================
Hits 96101 96101
- Misses 607 610 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cirq-core/cirq/ops/wait_gate.py
Outdated
| self._duration = ( | ||
| duration if isinstance(duration, value.Duration) else value.Duration(duration) | ||
| ) | ||
| if not self.duration._is_parameterized_() and self.duration.total_picos() < 0: |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
2 participants
This improves creation of WaitGate objects (e.g. from cirq.wait(nanos=10)) from about 27 micros to about 16 micros. While not a huge effect, this could have measurable savings for certain T1 and similar experiments that involve creation of lots of wait gates.