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

Adding layerwise folding as tutorial to mitiq. #1894

Merged
merged 11 commits into from
Jul 18, 2023

Conversation

vprusso
Copy link
Contributor

@vprusso vprusso commented Jun 30, 2023

  • Tutorial provided to showcase functionality for layerwise folding and error mitigation.

@github-actions
Copy link

Binder 👈 Launch a binder notebook on branch unitaryfund/mitiq/layerwise-folding-tutorial

@codecov
Copy link

codecov bot commented Jul 1, 2023

Codecov Report

Merging #1894 (546afb9) into master (3331c2a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1894   +/-   ##
=======================================
  Coverage   98.52%   98.52%           
=======================================
  Files          76       76           
  Lines        3520     3520           
=======================================
  Hits         3468     3468           
  Misses         52       52           

@vprusso
Copy link
Contributor Author

vprusso commented Jul 1, 2023

Not sure if I'm missing something, but all of the tests within the build/docs directory appear to be passing (link), but somehow the whole docs pipeline fails. Is there an obvious reason as to why this might be the case?

@nathanshammah
Copy link
Member

Thanks @vprusso! This is a great addition to the documentation.
Regarding the failing tests, it looks like CI doesn't like #step:6:349,

/home/runner/work/mitiq/mitiq/docs/source/examples/layerwise-folding.md:25: WARNING: 'myst' reference target not found: total-variational-distance-metric

/home/runner/work/mitiq/mitiq/docs/source/examples/layerwise-folding.md:26: WARNING: 'myst' reference target not found: impact-of-single-vs.-double-folding
/home/runner/work/mitiq/mitiq/docs/source/examples/layerwise-folding.md:28: WARNING: 'myst' reference target not found: #experiment:-global-folding-with-linear-extrapolation
/home/runner/work/mitiq/mitiq/docs/source/examples/layerwise-folding.md:29: WARNING: 'myst' reference target not found: #experiment:-layerwise-folding-with-linear-extrapolation

Can you please add the file to the index file, at examples.md? It would be great to include a thumbnail too, including a path for it in the conf.py and adding it to the _thumbnails folder. You can check the correct build of the docs here in the CI test list, looking at the RTD build.

@vprusso
Copy link
Contributor Author

vprusso commented Jul 3, 2023

Awesome. Thanks, @nathanshammah. We should be all ✅ now!

Copy link
Member

@andreamari andreamari left a comment

Choose a reason for hiding this comment

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

Thanks Vincent, very nice example!
Minor suggestions are given below.

docs/source/examples/layerwise-folding.md Outdated Show resolved Hide resolved
docs/source/examples/layerwise-folding.md Outdated Show resolved Hide resolved
docs/source/examples/layerwise-folding.md Outdated Show resolved Hide resolved
docs/source/examples/layerwise-folding.md Outdated Show resolved Hide resolved
docs/source/examples/layerwise-folding.md Outdated Show resolved Hide resolved
docs/source/examples/layerwise-folding.md Outdated Show resolved Hide resolved
@natestemen natestemen added this to the 0.28.0 milestone Jul 7, 2023
@vprusso
Copy link
Contributor Author

vprusso commented Jul 9, 2023

Thanks again for the comments, @andreamari !

These should all (hopefully) be addressed now. Thank you!

@vprusso
Copy link
Contributor Author

vprusso commented Jul 10, 2023

Thanks for the approval, @andreamari. Any qualms @natestemen , or would you say this is good to go?

Copy link
Member

@natestemen natestemen left a comment

Choose a reason for hiding this comment

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

Nice work Vincent, thanks for putting this together.

Biggest thing is just a minor comment about moving some of the motivational context to the beginning of the tutorial.

docs/source/examples/layerwise-folding.md Outdated Show resolved Hide resolved
docs/source/examples/layerwise-folding.md Outdated Show resolved Hide resolved
Comment on lines 282 to 289
So why consider this technique in this context? One reason is that applying
global folding will increase the length of the entire circuit while layerwise
folding on a subset of only the noisiest layers will increase the circuit by a
smaller factor.

If running a circuit on hardware is bottle-necked by the cost of running a long
circuit, this technique could potentially be used to arrive at a better result
(although not as good as global folding) but with less monetary cost.
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this motivational material should be moved to the top?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Done!

Copy link
Member

Choose a reason for hiding this comment

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

I see you moved it to the top in 28e0c5f, but wondering if we want remove it from the bottom as well, or it's okay that's it's duplicated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, I'm just a fool who left it in there twice. Removed, and thank you for the catch!

@vprusso
Copy link
Contributor Author

vprusso commented Jul 15, 2023

Thanks for the comments, @natestemen !

I believe they have been addressed. Let me know if there's anything else, but if not, I think it's good to go!

Copy link
Member

@natestemen natestemen left a comment

Choose a reason for hiding this comment

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

:shipit:

@vprusso
Copy link
Contributor Author

vprusso commented Jul 18, 2023

Awesome, thanks for the approval, @natestemen. I believe things are good (although it seems like the docs are failing for some odd reason). I know that they sometimes do this, so I'm not sure if this is just an artifact of something unrelated.

I attempted to rerun the docs build, but it still seems to fail. It mentions something about bsqkit (which I didn't touch in my MR):

/home/runner/work/mitiq/mitiq/docs/source/examples/bqskit.md: WARNING: Executing notebook failed: CellExecutionError [mystnb.exec]
/home/runner/work/mitiq/mitiq/docs/source/examples/bqskit.md: WARNING: Notebook exception traceback saved in: /home/runner/work/mitiq/mitiq/docs/build/html/reports/examples/bqskit.err.log [mystnb.exec]

Is it okay to merge or should I hold off?

@natestemen
Copy link
Member

Yup that's a problem that we are seeing across all PRs are the moment, so no need to worry about that. Sorry for the confusion. I'll let you hit the merge button when you want since it's fun :)

@vprusso
Copy link
Contributor Author

vprusso commented Jul 18, 2023

It is fun! :) Away it goes 🚀

@vprusso vprusso merged commit fc92819 into master Jul 18, 2023
20 of 21 checks passed
@vprusso vprusso deleted the layerwise-folding-tutorial branch July 18, 2023 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants