2189 remove unused folding functions #2289

jordandsullivan · 2024-04-12T00:39:53Z

Description

Removed zne.scaling.fold_gates_from_left and zne.scaling.fold_gates_from_right as they have not been widely used by the community.
Commented out calls to these functions and added a deprecation notice in the
Removed any imports and tests using these functions.
Modified tests which used parameterized inputs which included these functions.

Remaining question: do we want to eliminate the 'fold_method' argument from zne.scaling.folding._create_fold_mask function as explained in this comment?

License

I license this contribution under the terms of the GNU GPL, version 3 and grant Unitary Fund the right to provide additional permissions as described in section 7 of the GNU GPL, version 3.

Before opening the PR, please ensure you have completed the following where appropriate.

I added unit tests for new code.
I used type hints in function signatures.
I used Google-style docstrings for functions.
I updated the documentation where relevant.
Added myself / the copyright holder to the AUTHORS file

…e.scaling subpackage and in tests

…om_right in testing

codecov · 2024-04-12T00:54:26Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.30%. Comparing base (35139fe) to head (9220809).
Report is 9 commits behind head on main.

❗ Current head 9220809 differs from pull request most recent head ded961c. Consider uploading reports for the commit ded961c to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2289      +/-   ##
==========================================
+ Coverage   98.28%   98.30%   +0.02%     
==========================================
  Files          90       90              
  Lines        4263     4260       -3     
==========================================
- Hits         4190     4188       -2     
+ Misses         73       72       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mitiq/zne/scaling/folding.py

cosenal · 2024-04-12T19:23:00Z

docs/source/examples/mitiq-paper/mitiq-paper-codeblocks.md


-+++ {"id": "DRplhovffVfk"}
-
+<!-- #region id="DRplhovffVfk" -->


I wonder why there is so much noise in the diff of this file. It would be nice to have only a diff that matters.

I'm not an expert in Myst, but perhaps it's because I converted this to a jupyter notebook to run it and then converted it back to .md using jupytext? @natestemen Do you have insight on this?

That sounds like a good explanation.

I have never modified any of them myself, but I think it's worth putting some effort into finding (and documenting) a workflow that doesn't create this clutter. It will make reviews easier.

Your theory aligns with what I've seen in the past working with jupytext. If you wouldn't mind removing the unnecessary differences in this PR as a temporary workaround, it would be appreciated.

Correct me if I'm wrong here, but it looks like the .md version of this notebook in main is actually also messed up?

That is, it's displaying the info about Jupytext versions at the top and includes a bunch of lines like
+++ {"id": "DRplhovffVfk"}?

Also has collab info. Do we want all this present?

@natestemen This item should be the last thing. Not sure if we want to include fixing the formatting on this notebook in this PR or not, since it's present in main. The functional part is done.

It looks like you've done a bit of clean up in this PR, but lets keep the rest in a subsequent PR. Even though the markdown does render a little strangely here on GitHub, it looks fine on the live documentation, which is what matters most.

Cool. In that case should be ready to merge.

A formal approval is needed before merging. Now that you've added more commits since the first review, typically a second round of review would be initiated to ensure the new additions look good as well. To initiate another round of review you can request it using the "Re-request review" feature.

natestemen

Looking good! A few minor comments, but all the important stuff is done.

BTW can you ensure your GitHub commits are affiliated with your GitHub user?

docs/source/examples/mitiq-paper/mitiq-paper-codeblocks.md

docs/source/guide/zne-3-options.md

mitiq/zne/scaling/tests/test_folding.py

mitiq/zne/scaling/folding.py

jordandsullivan · 2024-04-17T19:24:18Z

Looking good! A few minor comments, but all the important stuff is done.

BTW can you ensure your GitHub commits are affiliated with your GitHub user?

Is this what you mean? If so, I'm not sure how it didn't get associated. It seems to still have my name but not be clickable?

natestemen · 2024-04-17T19:46:48Z

Yup exactly. Compare your screenshot to these commits.

From inspecting the commits on this branch it seems that you are using your UF email for email config which is a great idea, but it looks like GitHub does not that jordan@unitary.fund corresponds to @jordandsullivan. Once you add your UF email to GitHub [docs], then this should be sorted.

Here's one of the commits from this branch which you can see from running git log.

commit b72adf36e76190d3cd751c609cee0aa1a52c6e8c
Author: jordandsullivan <jordan@unitary.fund>
Date:   Thu Apr 11 13:51:12 2024 -0700
...

…(i.e. randomly)

Co-authored-by: nate stemen <nate@unitary.fund>

…sed arguments for fold_method

…xample not to use fold_gates_from_left

jordandsullivan · 2024-04-17T21:15:47Z

Yup exactly. Compare your screenshot to these commits.

From inspecting the commits on this branch it seems that you are using your UF email for email config which is a great idea, but it looks like GitHub does not that jordan@unitary.fund corresponds to @jordandsullivan. Once you add your UF email to GitHub [docs], then this should be sorted.

Here's one of the commits from this branch which you can see from running git log.
commit b72adf36e76190d3cd751c609cee0aa1a52c6e8c
Author: jordandsullivan <jordan@unitary.fund>
Date:   Thu Apr 11 13:51:12 2024 -0700
...

Gotcha thanks, I added my UF email address to my Github account so it should all be linked now.

cosenal · 2024-04-18T08:25:10Z

<commercial break>
I see validate checks are failing. This could mean that you haven't installed the ✨ fabulous mitiq git hooks ✨. You should! It's as easy as

make install-hooks

from your mitiq local folder.
<end of commercial break>

jordandsullivan · 2024-04-18T17:55:59Z

<commercial break> I see validate checks are failing. This could mean that you haven't installed the ✨ fabulous mitiq git hooks ✨. You should! It's as easy as
make install-hooks
from your mitiq local folder. <end of commercial break>

Ah, why thank you! I didn't realize you needed to install the hooks as well as having them in your .git-hooks file.

cosenal

LGTM

* Removed all instances of fold_from_left and fold_from_right in the zne.scaling subpackage and in tests * Updating typo in .gitignore * Formatting * removed remaining instances of fold_gates_from_left and fold_gates_from_right in testing * reverted accidentally modified .md notebook * formatting * Formatting for notebook md to myst * Remove fold_method as parameter for test * Update docstring to acurately reflect how folding indices are chosen (i.e. randomly) * Uncomment fold_gates_from_left call, as using directives now Co-authored-by: nate stemen <nate@unitary.fund> * Replace note with note directive Co-authored-by: nate stemen <nate@unitary.fund> * Removing the id outputs * Switching wording 'deprecated' to 'removed' * Remove parameterization for tests with only one test case, remove unused arguments for fold_method * Formatting * Remove fold_gates_from_left from pennylane demo * Previous commit msg was in error. This commit updates the pennylane example not to use fold_gates_from_left * Change notes to warnings, uncomment fold_gates_from_left and right code * Remove fold_gates_from_right from vqe-pennylane example, formatting * remove folding_method as optional argument to _create_fold_mask --------- Co-authored-by: nate stemen <nate@unitary.fund>

jordandsullivan added 5 commits April 11, 2024 13:51

Removed all instances of fold_from_left and fold_from_right in the zn…

b72adf3

…e.scaling subpackage and in tests

Updating typo in .gitignore

a3a766d

Formatting

0c38b34

removed remaining instances of fold_gates_from_left and fold_gates_fr…

0ab23ce

…om_right in testing

reverted accidentally modified .md notebook

8dac595

jordandsullivan linked an issue Apr 12, 2024 that may be closed by this pull request

Remove unused folding functions #2189

Closed

formatting

ac02d62

cosenal reviewed Apr 12, 2024

View reviewed changes

Formatting for notebook md to myst

6bbc1fa

natestemen reviewed Apr 16, 2024

View reviewed changes

jordandsullivan and others added 10 commits April 17, 2024 12:47

Remove fold_method as parameter for test

64fe322

Update docstring to acurately reflect how folding indices are chosen …

0c31347

…(i.e. randomly)

Uncomment fold_gates_from_left call, as using directives now

9c9d406

Co-authored-by: nate stemen <nate@unitary.fund>

Replace note with note directive

6e7f02e

Co-authored-by: nate stemen <nate@unitary.fund>

Removing the id outputs

7d6d1af

Switching wording 'deprecated' to 'removed'

421f17c

Remove parameterization for tests with only one test case, remove unu…

e66cf55

…sed arguments for fold_method

Formatting

3863825

Remove fold_gates_from_left from pennylane demo

9ba6b9e

Previous commit msg was in error. This commit updates the pennylane e…

3d8f8b4

…xample not to use fold_gates_from_left

Change notes to warnings, uncomment fold_gates_from_left and right code

dd9f14a

jordandsullivan added 2 commits April 18, 2024 11:08

Remove fold_gates_from_right from vqe-pennylane example, formatting

9220809

remove folding_method as optional argument to _create_fold_mask

ded961c

jordandsullivan requested a review from purva-thakre April 19, 2024 15:37

jordandsullivan added this to the v0.36.0 milestone Apr 19, 2024

cosenal approved these changes Apr 24, 2024

View reviewed changes

Merge branch 'main' into 2189-remove-unused-folding-functions

d4c5f94

jordandsullivan merged commit 42779ed into main Apr 25, 2024
14 of 15 checks passed

jordandsullivan deleted the 2189-remove-unused-folding-functions branch April 25, 2024 21:13

FarLab removed this from the v0.36.0 milestone Apr 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2189 remove unused folding functions #2289

2189 remove unused folding functions #2289

jordandsullivan commented Apr 12, 2024 •

edited

Loading

codecov bot commented Apr 12, 2024 •

edited

Loading

cosenal Apr 12, 2024

jordandsullivan Apr 16, 2024

cosenal Apr 16, 2024

natestemen Apr 16, 2024

jordandsullivan Apr 17, 2024 •

edited

Loading

jordandsullivan Apr 17, 2024

jordandsullivan Apr 18, 2024

natestemen Apr 18, 2024

jordandsullivan Apr 19, 2024

natestemen Apr 19, 2024

natestemen left a comment

jordandsullivan commented Apr 17, 2024

natestemen commented Apr 17, 2024

jordandsullivan commented Apr 17, 2024

cosenal commented Apr 18, 2024

jordandsullivan commented Apr 18, 2024

cosenal left a comment


		+++ {"id": "DRplhovffVfk"}

		<!-- #region id="DRplhovffVfk" -->

2189 remove unused folding functions #2289

2189 remove unused folding functions #2289

Conversation

jordandsullivan commented Apr 12, 2024 • edited Loading

Description

License

codecov bot commented Apr 12, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jordandsullivan Apr 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natestemen left a comment

Choose a reason for hiding this comment

jordandsullivan commented Apr 17, 2024

natestemen commented Apr 17, 2024

jordandsullivan commented Apr 17, 2024

cosenal commented Apr 18, 2024

jordandsullivan commented Apr 18, 2024

cosenal left a comment

Choose a reason for hiding this comment

jordandsullivan commented Apr 12, 2024 •

edited

Loading

codecov bot commented Apr 12, 2024 •

edited

Loading

jordandsullivan Apr 17, 2024 •

edited

Loading