Skip to content

Conversation

@NoureldinYosri
Copy link
Collaborator

@NoureldinYosri NoureldinYosri commented Feb 21, 2024

first step to fix #6440

cc: @eliottrosenberg

@CirqBot CirqBot added the size: M 50< lines changed <250 label Feb 21, 2024
@NoureldinYosri NoureldinYosri marked this pull request as ready for review February 21, 2024 20:26
@codecov
Copy link

codecov bot commented Feb 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.74%. Comparing base (598c84f) to head (1b855a9).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6472      +/-   ##
==========================================
- Coverage   97.75%   97.74%   -0.02%     
==========================================
  Files        1105     1105              
  Lines       94909    94925      +16     
==========================================
  Hits        92783    92783              
- Misses       2126     2142      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

}

message CouplerPulseGate{
optional FloatArg hold_time_ps = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add line comments. ps=pico seconds? Do we foresee a need for sub-nanosecond timing?

Copy link
Collaborator Author

@NoureldinYosri NoureldinYosri Feb 22, 2024

Choose a reason for hiding this comment

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

added. initially I was using ns, but when I wrote a test that used ps I got precision error. the test input was cirq.Duration(picos=1) and when the object got translated it had cirq.Duration(nanos=1.0000000..1e-3). so I'm using ps to avoid that.

@NoureldinYosri NoureldinYosri enabled auto-merge (squash) February 26, 2024 21:25
@NoureldinYosri NoureldinYosri merged commit 02d29a9 into main Feb 26, 2024
assert result.returncode == 0


@pytest.mark.skip(reason="fails due to boken dependendencies #6475")
Copy link
Collaborator

Choose a reason for hiding this comment

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

fyi - it is better to submit a separate tiny PR with this change only so that we keep a single topic per PR.

@pavoljuhas pavoljuhas deleted the coupler_pulse branch January 22, 2025 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: M 50< lines changed <250

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot serialize cirq_google.experimental.ops.coupler_pulse.CouplerPulse()

4 participants