Refactoring of NoisyOperation, NoisyBasis, OperationRepresentation #1712

andreamari · 2023-02-10T14:58:56Z

License

[ x] 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.

github-actions · 2023-02-10T14:59:09Z

👈 Launch a binder notebook on branch unitaryfund/mitiq/improve-pec-types

codecov · 2023-02-10T19:09:11Z

Codecov Report

Merging #1712 (0dbc4b2) into master (fe04e5e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1712   +/-   ##
=======================================
  Coverage   98.49%   98.49%           
=======================================
  Files          68       68           
  Lines        3186     3187    +1     
=======================================
+ Hits         3138     3139    +1     
  Misses         48       48

Impacted Files	Coverage Δ
mitiq/pec/types/__init__.py	`100.00% <ø> (ø)`
mitiq/pec/__init__.py	`100.00% <100.00%> (ø)`
mitiq/pec/representations/biased_noise.py	`100.00% <100.00%> (ø)`
mitiq/pec/representations/damping.py	`100.00% <100.00%> (ø)`
mitiq/pec/representations/depolarizing.py	`100.00% <100.00%> (ø)`
mitiq/pec/representations/optimal.py	`97.14% <100.00%> (-0.16%)`	⬇️
mitiq/pec/sampling.py	`100.00% <100.00%> (ø)`
mitiq/pec/types/types.py	`100.00% <100.00%> (ø)`
mitiq/calibration/settings.py	`100.00% <0.00%> (ø)`
mitiq/calibration/calibrator.py	`98.87% <0.00%> (+0.26%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

andreamari · 2023-02-23T18:31:29Z

@Misty-W and/or @natestemen , I am fighting with some silly style issues but all other tests are passing. So feel free to review this PR when you have some time.

natestemen

Looks pretty good to me, but I don't think I fully understand why switching from { op: coeff } to [ ops ] and [ coeffs ] is better. I know in the ticket you mentioned having an ordering is nice, but seeing as there is a tight connection between the operations and coefficients, it does seem natural to me to have them stored in a dictionary. Can you just provide a little more detail on what this achieves?

In terms of future plans, we maybe remove NoisyBasis altogether in the release after the current one?

natestemen · 2023-02-23T23:55:08Z

mitiq/pec/sampling.py

-    ideal, _ = convert_to_mitiq(ideal_operation)
+    ideal, native_type = convert_to_mitiq(ideal_operation)


So this function could take any QPROGRAM, but previously always returned cirq.Circuits. Is my understanding right?

I am not sure what type it was previously returning (since NoisyOperation.circuit was different) but, yes, it should return a QPROGRAM with the same input type.

Note also that an important update introduced here is that the qubits of the NoisyOperations are replaced by the qubits of the ideal operation that we aim to represent (the input argument of the function). This mapping is superfluous if NoisyOperation._is_qubit_dependent is True, but it is necessary when it is False.

I just added an inline suggestion to avoid useless conversions when NoisyOperation._is_qubit_dependent == True.

mitiq/pec/sampling.py

natestemen · 2023-02-24T00:04:50Z

mitiq/pec/types/__init__.py

 from mitiq.pec.types.types import (
-    NoisyBasis,
    NoisyOperation,
    OperationRepresentation,
+    NoisyBasis,
 )


Does changing the order of imports affect anything, or just reordering to show level of importance?

This is because I first removed NoisyBasis from Mitiq and then re-added a deprecated version of the NoisyBasis later. Reordering was not intentional.

To confirm, is the idea of re-adding just to warn the user that NoisyBasis is deprecated?

Yes. We can completely remove it in 1 or 2 milestones.

Technically, this is not a deprecation intended in the standard way, which would imply warning but still supporting NoisyBasis. In our case we just drop it and we give some info to the user with an error message.

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

andreamari · 2023-02-24T09:34:31Z

Looks pretty good to me, but I don't think I fully understand why switching from { op: coeff } to [ ops ] and [ coeffs ] is better. I know in the ticket you mentioned having an ordering is nice, but seeing as there is a tight connection between the operations and coefficients, it does seem natural to me to have them stored in a dictionary. Can you just provide a little more detail on what this achieves?

Some motivations:

[Main motivation] Using NoisyOperations as a keys of a dictionary, requires having a reliable and well-tested __hash__() function for the NoisyOperation class. We don't have one, so we have been using the default hash based on the memory id which is quite unsafe.
From a practical usage point of view, it's probably a matter of subjective preference. I like separated lists for representing objects in a vector space since one typically fixes the basis with a fixed ordering and, once this is done, one can represent objects via simple real vectors which can be easily transformed, optimized, sampled, etc... .
An example use case is when solving a numerical optimization problem to find the optimal coefficients. The optimal solution is a real vector of coefficients, but to what NoisyOperations do the coefficients correspond? This ambiguity is absent with the new approach.
Note that NoisyOperation.basis_expansion still exists and can be used to couple coefficients and basis elements.

mitiq/pec/sampling.py

natestemen

large cleaning of NosiyOperation

d7783ce

andreamari added 2 commits February 10, 2023 17:53

fix circuit vs native_circuit

205a41f

further improvements to NoisyOperation

55e31ee

test copy to increase coverage

6a030e6

andreamari mentioned this pull request Feb 14, 2023

Simplify noisy operation #1713

Merged

1 task

andreamari added 5 commits February 17, 2023 19:29

del NoisyBasis and basisexpansion, tests will fail

89d6c6e

fix tests and fix find_optimal_representation()

29cbb55

fix style

7a4cc12

update docs

93bc66c

actually remove NoisyBasis

bed7b3d

andreamari force-pushed the improve-pec-types branch from d858f43 to bed7b3d Compare February 21, 2023 14:14

andreamari marked this pull request as ready for review February 21, 2023 16:49

andreamari added 6 commits February 22, 2023 10:55

restore previous cirq style printing

f0700e0

fix is_qubit_dependent option and validate qubits

bb8fe37

Merge remote-tracking branch 'origin/master' into improve-pec-types

5db59c1

fix coverage

bdc4001

fix flake

5beee8f

fix format

fad8b4b

natestemen reviewed Feb 24, 2023

View reviewed changes

andreamari and others added 2 commits February 24, 2023 09:50

use dict(zip())

70bdd1d

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

fix flake again

9f7da3a

andreamari commented Feb 24, 2023

View reviewed changes

mitiq/pec/sampling.py Outdated Show resolved Hide resolved

Misty-W changed the title ~~Refactoring of NosiyOperation, NoisyBasis, OperationRepresentation~~ Refactoring of NoisyOperation, NoisyBasis, OperationRepresentation Feb 24, 2023

andreamari and others added 2 commits February 28, 2023 11:42

Map qubits only if necessary.

9613605

fix var name native_circuit --> native_circ

0dbc4b2

natestemen approved these changes Feb 28, 2023

View reviewed changes

natestemen added this pull request to the merge queue Feb 28, 2023

Merged via the queue into master with commit ca1620d Feb 28, 2023

Misty-W deleted the improve-pec-types branch February 28, 2023 21:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactoring of NoisyOperation, NoisyBasis, OperationRepresentation #1712

Refactoring of NoisyOperation, NoisyBasis, OperationRepresentation #1712

andreamari commented Feb 10, 2023 •

edited

github-actions bot commented Feb 10, 2023

codecov bot commented Feb 10, 2023 •

edited

andreamari commented Feb 23, 2023

natestemen left a comment

natestemen Feb 23, 2023

andreamari Feb 24, 2023

natestemen Feb 24, 2023

andreamari Feb 24, 2023

Misty-W Feb 24, 2023 •

edited

andreamari Feb 28, 2023 •

edited

andreamari commented Feb 24, 2023

natestemen left a comment

		ideal, _ = convert_to_mitiq(ideal_operation)
		ideal, native_type = convert_to_mitiq(ideal_operation)

Refactoring of NoisyOperation, NoisyBasis, OperationRepresentation #1712

Refactoring of NoisyOperation, NoisyBasis, OperationRepresentation #1712

Conversation

andreamari commented Feb 10, 2023 • edited

License

github-actions bot commented Feb 10, 2023

codecov bot commented Feb 10, 2023 • edited

Codecov Report

andreamari commented Feb 23, 2023

natestemen left a comment

Choose a reason for hiding this comment

natestemen Feb 23, 2023

Choose a reason for hiding this comment

andreamari Feb 24, 2023

Choose a reason for hiding this comment

natestemen Feb 24, 2023

Choose a reason for hiding this comment

andreamari Feb 24, 2023

Choose a reason for hiding this comment

Misty-W Feb 24, 2023 • edited

Choose a reason for hiding this comment

andreamari Feb 28, 2023 • edited

Choose a reason for hiding this comment

andreamari commented Feb 24, 2023

natestemen left a comment

Choose a reason for hiding this comment

andreamari commented Feb 10, 2023 •

edited

codecov bot commented Feb 10, 2023 •

edited

Misty-W Feb 24, 2023 •

edited

andreamari Feb 28, 2023 •

edited