LRE Executors #2499

purva-thakre · 2024-09-11T14:12:14Z

Description

Fixes #2370

List of todos:

Docstrings
Additional unit tests

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

codecov · 2024-09-11T14:26:00Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.72%. Comparing base (1a3705c) to head (21aeb4d).
Report is 6 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2499   +/-   ##
=======================================
  Coverage   98.71%   98.72%           
=======================================
  Files          89       90    +1     
  Lines        4131     4156   +25     
=======================================
+ Hits         4078     4103   +25     
  Misses         53       53

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

mitiq/lre/tests/test_lre.py

purva-thakre · 2024-09-13T10:14:29Z

This test does not fail locally: https://github.com/unitaryfund/mitiq/actions/runs/10847148259/job/30101612145?pr=2499#step:6:4556

Will dig into this.

natestemen

Nice work Purva! All your hard work makes this a nice and simple PR :)

Maybe we can do some debugging as to the num_chunks issue in tomorrows coding session.

mitiq/lre/lre.py

natestemen · 2024-09-23T23:43:20Z

mitiq/lre/lre.py

+    folding_method: Callable[
+        [Union[Any], float], Union[Any]
+    ] = fold_gates_at_random,


Let's follow what happens in ZNE here for consistency.

mitiq/mitiq/zne/zne.py

Line 70 in 6f912f6

scale_noise: Callable[[QPROGRAM, float], QPROGRAM] = fold_gates_at_random, # type: ignore [has-type]

I don't use the num_to_average param in LRE.

Sorry, I'm missing how num_to_average is related here. I'm just suggesting we typehint the folding method differently.

That's weird. I was viewing this suggestion locally in VS code. I interpreted 'follow what happens in ZNE for consistency' as use the typehint as well as the additional param.

Have you modified code above these lines? If so then it will probably change the location of the suggestion.

I hadn't modified anything in ZNE which is why it's weird that when you linked L70 in zne.py, L71 also showed up.

The location of the suggestion was not changed.

mitiq/lre/lre.py

mitiq/lre/tests/test_lre.py

cosenal

I left two comments, not blockers.

mitiq/lre/lre.py

cosenal · 2024-09-24T09:44:41Z

mitiq/lre/tests/test_lre.py

+    """Verify LRE decorators work as expected."""
+
+    @lre_decorator(degree=2, fold_multiplier=2)
+    def execute(circuit, noise_level=0.025):


Something more in the spirit of unit tests (and more future-proof) would be a test that only checks whether the correct functions are called, and doesn't actually run the circuit through the simulator. Here you are testing lots of things at the same time (including a Google's simulator), all of which could go wrong without giving much insights on the unit you are testing here, which is the behaviour of the new decorator.

@cosenal @natestemen Are either of you available for a quick call on Friday (excluding the community call)? I have been unsuccessful in trying to write a unit test that utilizes mock objects.

It's unclear to me what needs to be a mock object and what must have a pre-defined value.

mitiq/lre/tests/test_lre.py

cosenal · 2024-09-25T13:43:25Z

mitiq/lre/tests/test_lre.py

+    ideal_val = execute(test_cirq * 200, noise_level=0)
+    assert abs(ideal_val - noisy_val) > 0
+    lre_exp_val = execute_with_lre(
+        test_cirq * 200, execute, degree=2, fold_multiplier=2, num_chunks=5


I misled you by not creating a variable for test_cirq * 200 in y'day live coding*, but we do want one in the pull request.

*As @natestemen was bugging me about while I tried to hack things around as quick as possible 😅

I am going to use the mock testing module for this. As Nate noticed yesterday, test_cirq * 200 will increase the time taken by unit tests.

Discussed with Nate and Alessandro to skip this unit for when we get to #2512 in the future.

The code block that misbehaves when we have really large circuits is in one of the inference functions.

mitiq/mitiq/lre/inference/multivariate_richardson.py

Lines 165 to 182 in 6f912f6

except RuntimeWarning: # pragma: no cover

# taken from https://stackoverflow.com/a/19317237

warnings.warn( # pragma: no cover

"To account for overflow error, required determinant of "

+ "large sample matrix is calculated through "

+ "`np.linalg.slogdet`."

)

sign, logdet = np.linalg.slogdet( # pragma: no cover

input_sample_matrix

)

det = sign * np.exp(logdet) # pragma: no cover

if np.isinf(det):

raise ValueError( # pragma: no cover

"Determinant of sample matrix cannot be calculated as "

+ "the matrix is too large. Consider chunking your"

+ " input circuit. "

)

mitiq/lre/tests/test_lre.py

purva-thakre · 2024-09-27T20:45:20Z

I don't know why some of the tests are failing right now. Will rerun them in a couple of hours.

natestemen

A few minor changes requested, but looking good once those are addressed.

How long does it take to run the tests in test_lre.py?

mitiq/lre/lre.py

mitiq/lre/tests/test_lre.py

mitiq/lre/lre.py

purva-thakre · 2024-09-30T18:38:58Z

How long does it take to run the tests in test_lre.py?

I used pytest --durations=0 test_lre.py in mitiq/mitiq/lre/tests to get the following:

Chunking test where it is not expected to fail takes the longest.

https://stackoverflow.com/a/55095253

main functions without corrected mypy errors, unit tests and docstrings

6614641

purva-thakre force-pushed the lre_Executor branch 2 times, most recently from e328b9d to 52e0812 Compare September 11, 2024 19:10

add unit tests for the decorator - check for test coverage

3db2817

purva-thakre force-pushed the lre_Executor branch from 52e0812 to 3db2817 Compare September 11, 2024 19:36

purva-thakre added 3 commits September 12, 2024 07:42

fix #2500 (comment)

759fbc5

mypy - remove shots

2c35e74

more unit tests + docstrings

d0f0052

purva-thakre marked this pull request as ready for review September 13, 2024 09:41

purva-thakre requested review from natestemen and vprusso September 13, 2024 09:42

purva-thakre commented Sep 13, 2024

View reviewed changes

mitiq/lre/tests/test_lre.py Outdated Show resolved Hide resolved

purva-thakre added 3 commits September 13, 2024 04:45

dosctring args formatting

7708f50

fix #2499 (comment)

003d3a1

weird chunking failure

c970474

try chunking to 2

9146116

natestemen reviewed Sep 23, 2024

View reviewed changes

cosenal approved these changes Sep 24, 2024

View reviewed changes

num_chunks = 5 with test_cirq * 200

d926c5c

purva-thakre commented Sep 24, 2024

View reviewed changes

mitiq/lre/tests/test_lre.py Show resolved Hide resolved

cosenal reviewed Sep 25, 2024

View reviewed changes

purva-thakre added 2 commits September 25, 2024 17:51

push to check for test coverage

595360a

chunking failures

41d08f5

purva-thakre commented Sep 25, 2024

View reviewed changes

mitiq/lre/tests/test_lre.py Outdated Show resolved Hide resolved

purva-thakre added 2 commits September 27, 2024 11:36

split decorator unit test

ce46c58

cleanup

4561acb

purva-thakre requested a review from natestemen September 27, 2024 20:00

chunking failure again + docker failure

3d25cd2

purva-thakre force-pushed the lre_Executor branch from ae2f4fe to 3d25cd2 Compare September 27, 2024 20:41

natestemen approved these changes Sep 30, 2024

View reviewed changes

mitiq/lre/lre.py Outdated Show resolved Hide resolved

mitiq/lre/tests/test_lre.py Outdated Show resolved Hide resolved

mitiq/lre/tests/test_lre.py Outdated Show resolved Hide resolved

mitiq/lre/lre.py Outdated Show resolved Hide resolved

nate's suggestions

21aeb4d

purva-thakre merged commit 411e234 into main Oct 1, 2024
17 of 18 checks passed

purva-thakre deleted the lre_Executor branch October 1, 2024 19:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LRE Executors #2499

LRE Executors #2499

purva-thakre commented Sep 11, 2024 •

edited

Loading

codecov bot commented Sep 11, 2024 •

edited

Loading

purva-thakre commented Sep 13, 2024 •

edited

Loading

natestemen left a comment

natestemen Sep 23, 2024

purva-thakre Sep 25, 2024

natestemen Sep 25, 2024

purva-thakre Sep 25, 2024

natestemen Sep 25, 2024

purva-thakre Sep 25, 2024

cosenal left a comment

cosenal Sep 24, 2024

purva-thakre Sep 27, 2024 •

edited

Loading

cosenal Sep 25, 2024

purva-thakre Sep 25, 2024

purva-thakre Sep 27, 2024

purva-thakre commented Sep 27, 2024 •

edited

Loading

natestemen left a comment

purva-thakre commented Sep 30, 2024

	except RuntimeWarning: # pragma: no cover
	# taken from https://stackoverflow.com/a/19317237
	warnings.warn( # pragma: no cover
	"To account for overflow error, required determinant of "
	+ "large sample matrix is calculated through "
	+ "`np.linalg.slogdet`."
	)
	sign, logdet = np.linalg.slogdet( # pragma: no cover
	input_sample_matrix
	)
	det = sign * np.exp(logdet) # pragma: no cover

	if np.isinf(det):
	raise ValueError( # pragma: no cover
	"Determinant of sample matrix cannot be calculated as "
	+ "the matrix is too large. Consider chunking your"
	+ " input circuit. "
	)

LRE Executors #2499

LRE Executors #2499

Conversation

purva-thakre commented Sep 11, 2024 • edited Loading

Description

License

codecov bot commented Sep 11, 2024 • edited Loading

Codecov Report

purva-thakre commented Sep 13, 2024 • edited Loading

natestemen left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cosenal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

purva-thakre Sep 27, 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

purva-thakre commented Sep 27, 2024 • edited Loading

natestemen left a comment

Choose a reason for hiding this comment

purva-thakre commented Sep 30, 2024

purva-thakre commented Sep 11, 2024 •

edited

Loading

codecov bot commented Sep 11, 2024 •

edited

Loading

purva-thakre commented Sep 13, 2024 •

edited

Loading

purva-thakre Sep 27, 2024 •

edited

Loading

purva-thakre commented Sep 27, 2024 •

edited

Loading