Skip to content
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

Jump to bottom

ProjectorSum object for TFQ #4364

Merged
merged 67 commits into from
Aug 19, 2021
Merged

ProjectorSum object for TFQ #4364

merged 67 commits into from
Aug 19, 2021

Conversation

tonybruguier
Copy link
Contributor

This is a follow-up on #4331 which introduced ProjectorString objects. The present PR aims at adding ProjectorSum objects.

@tonybruguier
First retry at projectors
0339829
@tonybruguier
Delete soon-to-be-useless tests
a5ac063
@tonybruguier
Delete more useless tests
e428f58
@tonybruguier
Remove ability to use tuples
3f61ecd
@tonybruguier
Restrict to only integer, still TODO to make it sparse
6b13d7e
@tonybruguier
Now only using integers
9a1de0c
@tonybruguier
Some nits
420d2c7
@tonybruguier
nit
e419db3
@tonybruguier
Skeleton for ProjectorSum
b798ed7
@tonybruguier
JSON serialization
d4600d7
@tonybruguier
With multiplication
c648299
@tonybruguier
Operations
60b1846
@tonybruguier
Merge branch 'master' of github.com:quantumlib/Cirq into projector_st…
f8fc7b4 
…rings
@tonybruguier
Fixes
69965e3
@tonybruguier
Merge branch 'master' of github.com:quantumlib/Cirq into projector_st…
6027dbc 
…rings
@tonybruguier
Factor out ProjectorSum
6c0da89
@tonybruguier
sparse
33ff2c9
@tonybruguier
Delete dead code
89d2b35
@tonybruguier
Merge branch 'master' of github.com:quantumlib/Cirq into projector_st…
fd63c73 
…rings
@tonybruguier
Revert "Factor out ProjectorSum"
a9281df 
This reverts commit 6c0da89.
@tonybruguier
Fix unit test
7d3b8ed
@tonybruguier
matrix() return sparse matrix
887ae7f
@tonybruguier
Coeff, random state vector in testing
846af79
@tonybruguier
Only qubits
e8d8374
@tonybruguier
Faster expectations
df91f9e
@tonybruguier
docstrings
1cfecaf
@tonybruguier
Merge branch 'projector_strings' into projector_sums
123a02c
@tonybruguier
Fix more merge issues
23ff98e
@tonybruguier
Bug fix with coefficient
3db11fa
@tonybruguier
Merge branch 'master' of github.com:quantumlib/Cirq into projector_st…
20b2284 
…rings
@tonybruguier
Copy link
Contributor Author

Thanks, @MichaelBroughton

I tried to follow the code as much as possible. I also took the liberty of adding more tests. In particular, since I was confused earlier (and still) about whether or not to call .copy(), I decided to add tests to make sure the original objects were unmodified. I can, of course, remove these tests.

PTAL

@CirqBot CirqBot added size: XL lines changed >1000 size: L 250< lines changed <1000 and removed size: XL lines changed >1000 labels Aug 12, 2021
@tonybruguier
Copy link
Contributor Author

@MichaelBroughton
Now with the ability to add ProjectorString objects to ProjectorSum objects as you had suggested, I think.

MichaelBroughton
MichaelBroughton reviewed Aug 17, 2021
View reviewed changes
Copy link
Collaborator

@MichaelBroughton MichaelBroughton left a comment

Choose a reason for hiding this comment

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

This is looking pretty good. Just a few minor nits and expanding some of the tests a little bit then we should be good to merge.

cirq-core/cirq/ops/linear_combinations_test.py Outdated Show resolved Hide resolved
cirq-core/cirq/ops/linear_combinations.py Outdated Show resolved Hide resolved
cirq-core/cirq/ops/linear_combinations.py Outdated Show resolved Hide resolved
@tonybruguier
Copy link
Contributor Author

Thanks, @MichaelBroughton . PTAL when you have the time?

MichaelBroughton
MichaelBroughton approved these changes Aug 19, 2021
View reviewed changes
Copy link
Collaborator

@MichaelBroughton MichaelBroughton left a comment

Choose a reason for hiding this comment

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

LGTM

@MichaelBroughton MichaelBroughton added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Aug 19, 2021
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Aug 19, 2021
@CirqBot
Copy link
Collaborator

CirqBot commented Aug 19, 2021

Automerge cancelled: A required status check is not present.

Missing statuses: ['cla/google']

@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Aug 19, 2021
@MichaelBroughton MichaelBroughton added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Aug 19, 2021
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Aug 19, 2021
@CirqBot CirqBot merged commit 739808d into quantumlib:master Aug 19, 2021
@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Aug 19, 2021
@tonybruguier tonybruguier deleted the projector_sums branch August 19, 2021 03:20
@tonybruguier tonybruguier mentioned this pull request Aug 19, 2021
Open
@MichaelBroughton MichaelBroughton mentioned this pull request Aug 19, 2021
Open
@tonybruguier tonybruguier mentioned this pull request Aug 27, 2021
Merged
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
@tonybruguier @rht
ProjectorSum object for TFQ (quantumlib#4364)
765393f 
This is a follow-up on quantumlib#4331 which introduced ProjectorString objects. The present PR aims at adding ProjectorSum objects.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaelBroughton MichaelBroughton MichaelBroughton approved these changes

@cduck cduck Awaiting requested review from cduck

@vtomole vtomole Awaiting requested review from vtomole

@mpharrigan mpharrigan Awaiting requested review from mpharrigan

Assignees

@MichaelBroughton MichaelBroughton

Labels
area/expectation-value cla: yes Makes googlebot stop complaining. size: L 250< lines changed <1000
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tonybruguier @CirqBot @MichaelBroughton @mpharrigan