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

Deprecate CutQC #527

Merged
merged 24 commits into from
Apr 4, 2024
Merged

Deprecate CutQC #527

merged 24 commits into from
Apr 4, 2024

Conversation

ibrahim-shehzad
Copy link
Collaborator

Now that automated cut finding has been introduced (#520), we can go ahead and deprecate CutQC (#494).

@coveralls
Copy link

coveralls commented Apr 2, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 8560529644

Details

  • 49 of 49 (100.0%) changed or added relevant lines in 9 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.06%) to 95.433%
Totals Coverage Status
Change from base Build 8513967440: 0.06%
Covered Lines: 3448
Relevant Lines: 3613
💛 - Coveralls

@ibrahim-shehzad ibrahim-shehzad self-assigned this Apr 2, 2024
@ibrahim-shehzad ibrahim-shehzad added the cutqc Related to the wire cutting code based on the CutQC paper, arXiv:2012.02333 label Apr 2, 2024
@ibrahim-shehzad
Copy link
Collaborator Author

I put 0.8.0 as the release when cutqc will be removed. Don't know if we do micro releases and whether this should have been 0.7.1 instead.

@ibrahim-shehzad ibrahim-shehzad marked this pull request as ready for review April 2, 2024 14:25
@ibrahim-shehzad ibrahim-shehzad linked an issue Apr 2, 2024 that may be closed by this pull request
Deprecate CutQC #494
Closed
garrison
garrison reviewed Apr 2, 2024
View reviewed changes
test/cutting/test_cutqc.py Outdated Show resolved Hide resolved
@garrison
Copy link
Member

garrison commented Apr 2, 2024 •
edited
Loading

I put 0.8.0 as the release when cutqc will be removed. Don't know if we do micro releases and whether this should have been 0.7.1 instead.

Following semantic versioning, we should not remove code in a patchlevel release. We only do it in a minor release because we are pre-1.0.

So yes, 0.8.0 is indeed the soonest we should do the removal.

@caleb-johnson
Copy link
Collaborator

Should we do a single deprecation at the cutQC package level and forego the individual function deprecations?

@ibrahim-shehzad
Copy link
Collaborator Author

ibrahim-shehzad commented Apr 2, 2024 •
edited
Loading

Should we do a single deprecation at the cutQC package level and forego the individual function deprecations?

As Jim pointed out to me, the norm in qiskit tends to be to do both (cf. files here), which is why I was inclined to do this here.

caleb-johnson
caleb-johnson reviewed Apr 3, 2024
View reviewed changes
Copy link
Collaborator

@caleb-johnson caleb-johnson left a comment

Choose a reason for hiding this comment

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

We need a release note. We can be a little more explanatory here about the migration.

circuit_knitting/cutting/cutqc/__init__.py Outdated Show resolved Hide resolved
circuit_knitting/cutting/cutqc/dynamic_definition.py Outdated Show resolved Hide resolved
circuit_knitting/cutting/cutqc/dynamic_definition.py Outdated Show resolved Hide resolved
caleb-johnson
caleb-johnson reviewed Apr 3, 2024
View reviewed changes
docs/circuit_cutting/tutorials/04_automatic_cut_finding.ipynb Outdated
@@ -275,7 +275,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.8.16"
"version": "3.9.6"
Copy link
Collaborator

@caleb-johnson caleb-johnson Apr 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Can probably restore this file from main and remove it from the diff here

Copy link
Member

Choose a reason for hiding this comment

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

This is not actually resolved, but I won't hold up this PR for this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, should be good now: 2d2c553

caleb-johnson
caleb-johnson previously approved these changes Apr 3, 2024
View reviewed changes
Copy link
Collaborator

@caleb-johnson caleb-johnson left a comment

Choose a reason for hiding this comment

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

This all looks good to me. Thanks!

garrison and others added 4 commits April 4, 2024 10:00
@garrison
Move the deprecated imports inside the test functions
2b9aff8 
Otherwise, a deprecation warning will show any time the tests are run
@garrison
Skip the doctests in metrics.py
66b2425 
Otherwise we see the following when we run tests
```
circuit_knitting/utils/metrics.py::circuit_knitting.utils.metrics.cross_entropy
  <doctest circuit_knitting.utils.metrics.cross_entropy[0]>:1: DeprecationWarning: The function ``circuit_knitting.utils.metrics.cross_entropy()`` is deprecated as of circuit-knitting-toolbox 0.7.0. It will be removed Circuit knitting toolbox 0.8.0 release.

circuit_knitting/utils/metrics.py::circuit_knitting.utils.metrics.MSE
  <doctest circuit_knitting.utils.metrics.MSE[0]>:1: DeprecationWarning: The function ``circuit_knitting.utils.metrics.MSE()`` is deprecated as of circuit-knitting-toolbox 0.7.0. It will be removed Circuit knitting toolbox 0.8.0 release.

circuit_knitting/utils/metrics.py::circuit_knitting.utils.metrics.MAPE
  <doctest circuit_knitting.utils.metrics.MAPE[0]>:1: DeprecationWarning: The function ``circuit_knitting.utils.metrics.MAPE()`` is deprecated as of circuit-knitting-toolbox 0.7.0. It will be removed Circuit knitting toolbox 0.8.0 release.

circuit_knitting/utils/metrics.py::circuit_knitting.utils.metrics.chi2_distance
  <doctest circuit_knitting.utils.metrics.chi2_distance[0]>:1: DeprecationWarning: The function ``circuit_knitting.utils.metrics.chi2_distance()`` is deprecated as of circuit-knitting-toolbox 0.7.0. It will be removed Circuit knitting toolbox 0.8.0 release.

circuit_knitting/utils/metrics.py::circuit_knitting.utils.metrics.chi2_distance
  <doctest circuit_knitting.utils.metrics.chi2_distance[1]>:1: DeprecationWarning: The function ``circuit_knitting.utils.metrics.chi2_distance()`` is deprecated as of circuit-knitting-toolbox 0.7.0. It will be removed Circuit knitting toolbox 0.8.0 release.

circuit_knitting/utils/metrics.py::circuit_knitting.utils.metrics.HOP
  <doctest circuit_knitting.utils.metrics.HOP[0]>:1: DeprecationWarning: The function ``circuit_knitting.utils.metrics.HOP()`` is deprecated as of circuit-knitting-toolbox 0.7.0. It will be removed Circuit knitting toolbox 0.8.0 release.
```
@garrison
Also avoid importing cutqc during test discovery
c6bee07 
Otherwise, we will see a deprecation warning
@ibrahim-shehzad
Remove warnings from private methods, edit release note and removal t…
6c57712 
…imeline string.
garrison
garrison reviewed Apr 4, 2024
View reviewed changes
tox.ini Outdated Show resolved Hide resolved
garrison
garrison reviewed Apr 4, 2024
View reviewed changes
circuit_knitting/cutting/cutqc/__init__.py Outdated Show resolved Hide resolved
circuit_knitting/cutting/cutqc/dynamic_definition.py Outdated Show resolved Hide resolved
circuit_knitting/cutting/cutqc/__init__.py Outdated Show resolved Hide resolved
garrison
garrison reviewed Apr 4, 2024
View reviewed changes
tox.ini Outdated Show resolved Hide resolved
@garrison garrison mentioned this pull request Apr 4, 2024
Closed
6 tasks
garrison
garrison previously approved these changes Apr 4, 2024
View reviewed changes
Copy link
Member

@garrison garrison left a comment

Choose a reason for hiding this comment

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

Thanks, this looks great

garrison
garrison approved these changes Apr 4, 2024
View reviewed changes
Copy link
Member

@garrison garrison left a comment

Choose a reason for hiding this comment

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

thanks again!

@ibrahim-shehzad ibrahim-shehzad merged commit 6db5cbd into main Apr 4, 2024
11 checks passed
@ibrahim-shehzad ibrahim-shehzad deleted the deprecate-cutqc branch April 4, 2024 20:05
@garrison garrison mentioned this pull request Apr 9, 2024
Merged
1 task
@garrison garrison mentioned this pull request May 22, 2024
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garrison garrison garrison approved these changes

@caleb-johnson caleb-johnson caleb-johnson left review comments

Assignees

@ibrahim-shehzad ibrahim-shehzad

Labels
cutqc Related to the wire cutting code based on the CutQC paper, arXiv:2012.02333
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Deprecate CutQC
4 participants
@ibrahim-shehzad @coveralls @garrison @caleb-johnson