Skip to content

Deprecate MergeNQubitGates optimizer #5653

Merged
CirqBot merged 5 commits intomasterfrom
deprecate_merge_n_qubit_gates
Jul 7, 2022
Merged

Deprecate MergeNQubitGates optimizer #5653
CirqBot merged 5 commits intomasterfrom
deprecate_merge_n_qubit_gates

Conversation

@tanujkhattar
Copy link
Copy Markdown
Collaborator

@tanujkhattar tanujkhattar commented Jun 30, 2022

Part of #5028

@tanujkhattar tanujkhattar requested review from a team, cduck and vtomole as code owners June 30, 2022 00:22
@tanujkhattar tanujkhattar requested a review from pavoljuhas June 30, 2022 00:22
@review-notebook-app
Copy link
Copy Markdown

review-notebook-app bot commented Jun 30, 2022

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@CirqBot CirqBot added the size: S 10< lines changed <50 label Jun 30, 2022
vtomole
vtomole reviewed Jul 6, 2022
View reviewed changes
Comment thread cirq-core/cirq/contrib/quimb/grid_circuits.py


@cirq._compat.deprecated_class(deadline='v0.16', fix="Use cirq.merge_k_qubit_unitaries")
class MergeNQubitGates(cirq.PointOptimizer):
Copy link
Copy Markdown
Collaborator

@vtomole vtomole Jul 6, 2022

Choose a reason for hiding this comment

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

This class does more than cirq.merge_k_qubit_unitaries. It calls off to cirq.drop_negligible_operations and cirq.drop_empty_moments as well. Will existing tests for this optimizer pass if only cirq.merge_k_qubit_unitaries is called?

Copy link
Copy Markdown
Collaborator Author

@tanujkhattar tanujkhattar Jul 6, 2022

Choose a reason for hiding this comment

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

Can you point me to where MergeNQubitGates optimizer calls off to cirq.drop_negligible_operations and cirq.drop_empty_moments ?

Also, the call sites calling MergeNQubitGates have been updated to call cirq.merge_k_qubit_unitaries and no tests fail right now. But to answer your specific question, cirq.merge_k_qubit_unitaries does not call cirq.drop_negligible_operations and cirq.drop_empty_moments by default.

Copy link
Copy Markdown
Collaborator

@vtomole vtomole Jul 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Whoops, i mistook this for simplify_expectation_value_circuit that's after this.

vtomole
vtomole approved these changes Jul 7, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@vtomole vtomole left a comment

Choose a reason for hiding this comment

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

LGTM

@tanujkhattar tanujkhattar added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jul 7, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jul 7, 2022
@CirqBot CirqBot merged commit b592ae7 into master Jul 7, 2022
@CirqBot CirqBot removed the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jul 7, 2022
@CirqBot CirqBot deleted the deprecate_merge_n_qubit_gates branch July 7, 2022 01:32
@CirqBot CirqBot removed the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jul 7, 2022
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
@tanujkhattar @rht
Deprecate MergeNQubitGates optimizer (quantumlib#5653)
98e5024 
Part of quantumlib#5028
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
@tanujkhattar
Deprecate MergeNQubitGates optimizer (quantumlib#5653)
1020c82 
Part of quantumlib#5028
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vtomole vtomole vtomole approved these changes

@cduck cduck Awaiting requested review from cduck

@pavoljuhas pavoljuhas Awaiting requested review from pavoljuhas pavoljuhas was automatically assigned from quantumlib/cirq-maintainers

Assignees

No one assigned

Labels

size: S 10< lines changed <50

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tanujkhattar @vtomole @CirqBot