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.

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

Remove deprecated ArithmeticOperation #5579

Merged
merged 18 commits into from
Jul 7, 2022
Merged

Conversation

daxfohl
Copy link
Contributor

@daxfohl daxfohl commented Jun 22, 2022

@daxfohl daxfohl requested review from a team, vtomole and cduck as code owners June 22, 2022 21:57
@daxfohl daxfohl requested a review from pavoljuhas June 22, 2022 21:57
@CirqBot CirqBot added the size: L 250< lines changed <1000 label Jun 22, 2022
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 after fix on shors.ipynb

@daxfohl
Copy link
Contributor Author

daxfohl commented Jun 22, 2022

Oof, that looks nontrivial (at least for me -- I'd need to ramp up on how to work with ipynb files, which I don't have time for right now). Anybody who wants to fork from here, I know there's a shor.py that probably had the same change when I originally deprecated ArithmeticOperation: https://github.com/quantumlib/Cirq/pull/4702/files#diff-9d5e940cf487ce1f28d94c8a32b7c4c2a5da812e72b48aa6e5db733e3f511066

@daxfohl
Copy link
Contributor Author

daxfohl commented Jun 22, 2022

Closing PR since I won't be able to finish it.

@daxfohl daxfohl closed this Jun 22, 2022
@daxfohl daxfohl reopened this Jun 23, 2022
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@daxfohl
Copy link
Contributor Author

daxfohl commented Jun 23, 2022

@MichaelBroughton I figured out how to do this. However I'm getting an error saying cirq.ArithmeticGate is not defined. But it is. And it works locally. Any idea?

@pavoljuhas
Copy link
Collaborator

However I'm getting an error saying cirq.ArithmeticGate is not defined. But it is. And it works locally. Any idea?

I am new to the notebook test machinery, but the test may be running with a released cirq.
Perhaps the shor.ipynb notebook needs to be added to NOTEBOOKS_DEPENDING_ON_UNRELEASED_FEATURES -

NOTEBOOKS_DEPENDING_ON_UNRELEASED_FEATURES: List[str] = [
# Hardcoded qubit placement
'docs/google/qubit-placement.ipynb',
# get_qcs_objects_for_notebook
'docs/noise/calibration_api.ipynb',
'docs/tutorials/google/colab.ipynb',
'docs/tutorials/google/identifying_hardware_changes.ipynb',
'docs/tutorials/google/echoes.ipynb',
'docs/noise/floquet_calibration_example.ipynb',
'docs/tutorials/google/spin_echoes.ipynb',
'docs/tutorials/google/start.ipynb',
'docs/tutorials/google/visualizing_calibration_metrics.ipynb',
'docs/noise/qcvv/xeb_calibration_example.ipynb',
'docs/named_topologies.ipynb',
'docs/representing_noise.ipynb',
'docs/start/intro.ipynb',
]

@daxfohl
Copy link
Contributor Author

daxfohl commented Jun 23, 2022

@MichaelBroughton @pavoljuhas It turns out this is not ready to be removed. #4702 was started in November but did not get merged until May, which was after 14.1 went out. These deprecations need to be bumped to 0.16 instead.

@daxfohl daxfohl closed this Jun 23, 2022
CirqBot pushed a commit that referenced this pull request Jun 24, 2022
#4702 was started in November but did not get merged until May, which was after 14.1 went out. These deprecations need to be bumped to 0.16 instead of 0.15.

Once 0.15 goes out then I can reopen #5579 and delete these.
@daxfohl daxfohl reopened this Jun 29, 2022
@daxfohl
Copy link
Contributor Author

daxfohl commented Jun 29, 2022

@MichaelBroughton v0.15 is out, so reopening.

docs/experiments/shor.ipynb Outdated Show resolved Hide resolved
@MichaelBroughton MichaelBroughton 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 4b6ee22 into quantumlib:master Jul 7, 2022
@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 Jul 7, 2022
@daxfohl daxfohl deleted the dep branch August 12, 2022 05:15
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
quantumlib#4702 was started in November but did not get merged until May, which was after 14.1 went out. These deprecations need to be bumped to 0.16 instead of 0.15.

Once 0.15 goes out then I can reopen quantumlib#5579 and delete these.
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
quantumlib#4702 was started in November but did not get merged until May, which was after 14.1 went out. These deprecations need to be bumped to 0.16 instead of 0.15.

Once 0.15 goes out then I can reopen quantumlib#5579 and delete these.
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: L 250< lines changed <1000
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants