Skip to content

Conversation

@daxfohl
Copy link
Collaborator

@daxfohl daxfohl commented Mar 2, 2025

Fixes #7102. No longer coerces x/row/col to int. Wraps hash expression with hash instead.

On my laptop it's also a tiny bit faster. It creates 10,000,000 LineQubits in 2.1s; old code took 2.5s.

Also, note that I left all the type annotations int, which is what they have always been. I looked into changing these into numbers.Rational or SupportsFloat or something, but it's too invasive. Lots of code seems to assume integral x/row/col values, and mypy blows up when you try to expand it.

@daxfohl daxfohl requested review from a team and vtomole as code owners March 2, 2025 14:13
@daxfohl daxfohl requested a review from MichaelBroughton March 2, 2025 14:13
@CirqBot CirqBot added the size: S 10< lines changed <50 label Mar 2, 2025
@codecov
Copy link

codecov bot commented Mar 2, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.15%. Comparing base (2ad4136) to head (25c8ab0).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7110      +/-   ##
==========================================
- Coverage   98.15%   98.15%   -0.01%     
==========================================
  Files        1089     1089              
  Lines       95251    95255       +4     
==========================================
- Hits        93497    93495       -2     
- Misses       1754     1760       +6     

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

@daxfohl daxfohl requested review from dstrain115 and pavoljuhas March 2, 2025 18:25
@daxfohl daxfohl changed the title Allow non-integral line/grid qubits [Bugfix] Re-allow non-integral line/grid qubits Mar 3, 2025
Copy link
Collaborator

@dstrain115 dstrain115 left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

Copy link
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

LGTM with a couple of small comments. Thank you!

@daxfohl daxfohl enabled auto-merge March 3, 2025 21:53
@daxfohl daxfohl added this pull request to the merge queue Mar 3, 2025
Merged via the queue into quantumlib:main with commit 6ff3d66 Mar 3, 2025
38 checks passed
@daxfohl daxfohl deleted the fix-qid-hash branch March 3, 2025 22:09
BichengYing pushed a commit to BichengYing/Cirq that referenced this pull request Jun 20, 2025
* Allow non-integral line/grid qubits

* tests

* fix numpy overflow

* fix tests

* fix tests on windows

* Address PR comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: S 10< lines changed <50

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression for cirq.GridQubit with float indices

4 participants