Remove greedy recursion #537

ibrahim-shehzad · 2024-04-08T16:50:41Z

This PR aims to resolve #536, by getting rid of recursion inside the greedy_best_first_search algorithm that is used to warm start the search performed by the cut-finder. I have replaced recursion with a simple while loop and have verified that the issues reported to us for large QAOA circuits (which led to #536) are now avoided. Since these circuits have large widths, the algorithm takes a while to finish running and any direct tests on these circuits would significantly slow down the overall testing process. This is why I have not added any new tests at this stage.

Co-authored-by: Jim Garrison <[email protected]>

Otherwise, a deprecation warning will show any time the tests are run

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. ```

Otherwise, we will see a deprecation warning

…imeline string.

…s/circuit-knitting-toolbox into deprecate-cutqc

Co-authored-by: Jim Garrison <[email protected]>

coveralls · 2024-04-08T16:53:53Z

Pull Request Test Coverage Report for Build 8656075716

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 95.433%

Totals
Change from base Build 8560627750:	0.0%
Covered Lines:	3448
Relevant Lines:	3613

💛 - Coveralls

ibrahim-shehzad · 2024-04-08T16:56:59Z

With pieces of older git history showing up above, it looks like I may have (accidentally) created this branch locally from the deprecate-cutqc branch, instead of from main. Let me know if this is a problem and I should undo this and start over.

caleb-johnson · 2024-04-08T20:44:41Z

With pieces of older git history showing up above, it looks like I may have (accidentally) created this branch locally from the deprecate-cutqc branch, instead of from main. Let me know if this is a problem and I should undo this and start over.

@ibrahim-shehzad This can happen if you have commits from a feature branch and then those commits are squashed when that branch is merged. Git doesnt know what to do so it keeps things in the diff that are not actual changes. I think you can just checkout the main versions of these files and then commit and push those files and it'll update the diff

circuit_knitting/cutting/cut_finding/cco_utils.py

caleb-johnson · 2024-04-08T21:17:17Z

This looks nice, and it appears to implement the same logic. If you could just say something about what the conditions are when this loop should end, I'd like to consider if there are cases which could cause this to run forever.

caleb-johnson · 2024-04-09T15:00:53Z

circuit_knitting/cutting/cut_finding/cco_utils.py

+        else:
+            # avoid using recursion to stay clear of any risk of stack overflow.
+            state = best[-1]
+            continue


If you don't take my suggestion above, I think we can remove this continue anyway. I don't think it does anything. The while loop will evaluate its condition next regardless of whether this is here.

circuit_knitting/cutting/cut_finding/cco_utils.py

caleb-johnson

This looks good to me. A couple of small style suggestions, but it's fine to merge as-is IMO. Thanks!

circuit_knitting/cutting/cut_finding/cco_utils.py

ibrahim-shehzad and others added 25 commits April 2, 2024 09:51

deprecate cutqc

67386f1

Deprecate conversion and metrics.

ce89dbe

Remove low coverage line from tox.ini

6550136

Update test_cutqc.py

659cd83

Co-authored-by: Jim Garrison <[email protected]>

Update deprecation message

588aaf2

Co-authored-by: Jim Garrison <[email protected]>

change package name in deprecation messages

1d17be6

snapshot

4a388d3

Edit deprecation messages

08319c3

Remove extraneous backticks

659b8b4

add release note

b66926a

Edit release note

c8b810f

Update deprecation message in init file

e90fd4d

Co-authored-by: Jim Garrison <[email protected]>

Move the deprecated imports inside the test functions

2b9aff8

Otherwise, a deprecation warning will show any time the tests are run

Also avoid importing cutqc during test discovery

c6bee07

Otherwise, we will see a deprecation warning

Remove warnings from private methods, edit release note and removal t…

6c57712

…imeline string.

Merge branch 'deprecate-cutqc' of https://github.com/Qiskit-Extension…

601628d

…s/circuit-knitting-toolbox into deprecate-cutqc

Add ignore flags

dc31577

Correct typo in deprecation message

1497412

Co-authored-by: Jim Garrison <[email protected]>

Edit removal timeline field

fd5e44e

Remove trailing space

a9e1bb4

Double backticks

3f7142b

Add backticks around module name

735d9f1

restore notebook

2d2c553

Initiate greedy recursion removal

3b8729a

ibrahim-shehzad added the cut finder Related to the automatic cut finder label Apr 8, 2024

ibrahim-shehzad self-assigned this Apr 8, 2024

Remove notebook from diff

1c20633

white spaces

230c338

ibrahim-shehzad requested a review from caleb-johnson April 8, 2024 17:04

Merge branch 'main' into remove-greedy-recursion

2a766db

caleb-johnson reviewed Apr 8, 2024

View reviewed changes

circuit_knitting/cutting/cut_finding/cco_utils.py Outdated Show resolved Hide resolved

ibrahim-shehzad added the bug Something isn't working label Apr 8, 2024

ibrahim-shehzad requested a review from garrison April 9, 2024 13:45

caleb-johnson reviewed Apr 9, 2024

View reviewed changes

circuit_knitting/cutting/cut_finding/cco_utils.py Outdated Show resolved Hide resolved

garrison added this to the 0.7.0 milestone Apr 9, 2024

refactor while loop

dc43c78

garrison reviewed Apr 10, 2024

View reviewed changes

circuit_knitting/cutting/cut_finding/cco_utils.py Outdated Show resolved Hide resolved

Remove redundant lines from while loop

183c13a

caleb-johnson approved these changes Apr 12, 2024

View reviewed changes

circuit_knitting/cutting/cut_finding/cco_utils.py Show resolved Hide resolved

circuit_knitting/cutting/cut_finding/cco_utils.py Show resolved Hide resolved

ibrahim-shehzad merged commit 4ab172a into main Apr 12, 2024
11 checks passed

ibrahim-shehzad deleted the remove-greedy-recursion branch April 12, 2024 15:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove greedy recursion #537

Remove greedy recursion #537

ibrahim-shehzad commented Apr 8, 2024

coveralls commented Apr 8, 2024 •

edited

Loading

ibrahim-shehzad commented Apr 8, 2024 •

edited

Loading

caleb-johnson commented Apr 8, 2024

caleb-johnson commented Apr 8, 2024

caleb-johnson Apr 9, 2024 •

edited

Loading

caleb-johnson left a comment

Remove greedy recursion #537

Remove greedy recursion #537

Conversation

ibrahim-shehzad commented Apr 8, 2024

coveralls commented Apr 8, 2024 • edited Loading

Pull Request Test Coverage Report for Build 8656075716

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

ibrahim-shehzad commented Apr 8, 2024 • edited Loading

caleb-johnson commented Apr 8, 2024

caleb-johnson commented Apr 8, 2024

caleb-johnson Apr 9, 2024 • edited Loading

Choose a reason for hiding this comment

caleb-johnson left a comment

Choose a reason for hiding this comment

coveralls commented Apr 8, 2024 •

edited

Loading

ibrahim-shehzad commented Apr 8, 2024 •

edited

Loading

caleb-johnson Apr 9, 2024 •

edited

Loading