Bugfix reparameterize #480

juangmendoza19 · 2024-08-23T18:12:24Z

Addressing issue #425.

convert() function has been changed for POVM elements and state preparation. They now support proper conversion from full TP to GLND parameterization. The optimization is now done over non-gauge directions, which are found through Jacobians.

This function still needs to be changed to appropriately handle other reparameterizations.

Lastly, this is not guaranteed to work for POVMs, just because the number of degrees of freedom of a POVM can be greater than the number of error generators that span TP maps for a given number of qubits. By pigeon hole principle, we may not be able to find a description for a POVM consisting of Ideal_POVM + error generators. There is an assertion to try to catch this.

…s valid GLND representation for povm

sserita

Just bookkeeping: Not ready for review, rerequest from me when ready.

…e subset of models GLND, CPTPLND, H+S, S, full TP all to all when converting models with no noise. There is currently no support for converting to the same type for many of these

…ances

…re failing for some reason

juangmendoza19 · 2025-02-20T23:24:38Z

pygsti/baseobjs/errorgenbasis.py

+        TODO: 2 qubit labels returning None when label does exist in self.labels. 
+        '(support, left_support) not in self._offsets[eetype]' returning True incorrectly
+
+


The fancy code below (now commented out) to find the index of a label was not working for 2 qubits. Temporary easy solution implemented by just calling index(). This was creating problems during the creation of 2 qubit FOGI models

Pretty sure that I implemented a similar fix for this on PR #538. Making note of this in case there are merge conflicts that need to eventually get resolved here.

juangmendoza19 · 2025-02-20T23:27:11Z

pygsti/modelmembers/povms/__init__.py

+                            if ideal_effect is not None:
+                                base_items.append((lbl, convert_effect(ideal_effect, 'static', basis, ideal_effect, flatten_structure)))
+                            else:
+                                base_items.append((lbl, convert_effect(vec, 'static', basis, idl.get(lbl, None), flatten_structure)))


This is code to properly collect the ideal POVM effects. Before, the ideal_effect passed in was being ignored

juangmendoza19 · 2025-02-20T23:31:10Z

pygsti/modelmembers/povms/__init__.py

+                    raise ValueError("Failed to find an errorgen such that <ideal|exp(errorgen) = <effect|")
+                errgen_vec = _np.linalg.pinv(phys_directions)  @ soln.x
+                errorgen.from_vector(errgen_vec)
+


Code added to find errgen parameterization for POVMs based on optimization, just like state prep was being done before, but for some reason was never implemented on POVMs. Note that the optimization variations are done over a POVM space that lacks the dumb-gauge/metagauge. To find that space we wrote the function calc_physical_subspace which does a numerical calculation of the Jacobian with respect to POVM parameters.

juangmendoza19 · 2025-02-20T23:37:36Z

pygsti/modelmembers/states/__init__.py

+                    phys_directions = calc_physical_subspace(dense_state)
+
+                    #We use optimization to find the best error generator representation
+                    #we only vary physical directions, not independent error generators


Just as explained in the POVM init file, we added a function to identify the dumb-gauge/metagauge and move orthogonally to it. This allows for better optimization.

juangmendoza19 · 2025-02-20T23:38:24Z

pygsti/modelmembers/states/__init__.py

+                        return _np.linalg.norm(proc_matrix @ dense_st - dense_state) + cptp_penalty * sum_of_negative_choi_eigenvalues_gate(proc_matrix, dense_basis)
+
+                    soln = _spo.minimize(_objfn, _np.zeros(len(phys_directions), 'd'), method="Nelder-Mead", options={},
+                                         tol=1e-13)  # , callback=callback)


the objective function to be optimized over was modified to work with linear combinations of error generators which lack these dumb-gauge degrees of freedom

juangmendoza19 · 2025-02-20T23:39:20Z

pygsti/models/explicitmodel.py

@@ -340,7 +340,7 @@ def __getitem__(self, label):
            raise KeyError("Key %s has an invalid prefix" % label)

    def convert_members_inplace(self, to_type, categories_to_convert='all', labels_to_convert='all',
-                                ideal_model=None, flatten_structure=False, set_default_gauge_group=False, cptp_truncation_tol= 1e-6):
+                                ideal_model=None, flatten_structure=False, set_default_gauge_group=False, cptp_truncation_tol= 1e-7, spam_cptp_penalty = 1e-6):


added piping for the cptp penalty factor to be used in the optimization within the SPAM convert functions discussed above

juangmendoza19 · 2025-02-20T23:40:48Z

pygsti/tools/jamiolkowski.py

+    evals = _np.linalg.eigvals(J)  # could use eigvalsh, but wary of this since eigh can be wrong...
+    for ev in evals:
+            if ev.real < 0: sumOfNeg -= ev.real
+    return sumOfNeg


Created a function to sum the negative choi eigenvalues of a single gate to be used in our optimization functions to measure CPTPness

Catching up this branch with develop. Specifically I am trying to see if this fixes some unit tests that are currently failing.

juangmendoza19 · 2025-02-22T17:21:10Z

All the code that was changed which does not have a comment attached was a result of merging the develop branch.

coreyostrove · 2025-02-23T03:07:58Z

The commits coming from merging in develop shouldn't be showing up, so something got glitched out here. I can confirm that when I just went into the new PR creation interface and looked at the diff that was created for merging in this branch into develop only the new commits related to these bugfixes showed up. This might be the sort of thing that fixes itself if you switch the merge target to something other than develop (e.g. master) and then back again. It doesn't look like I have the permissions to do that on my end, so could either @juangmendoza19 or someone else with the requisite permissions give that a shot?

coreyostrove

See my inline comments and questions below. Thanks!

coreyostrove · 2025-02-23T18:47:03Z

pygsti/baseobjs/errorgenbasis.py

+        TODO: 2 qubit labels returning None when label does exist in self.labels. 
+        '(support, left_support) not in self._offsets[eetype]' returning True incorrectly
+
+


Pretty sure that I implemented a similar fix for this on PR #538. Making note of this in case there are merge conflicts that need to eventually get resolved here.

coreyostrove · 2025-02-23T18:55:31Z

pygsti/modelmembers/povms/__init__.py

@@ -327,13 +331,19 @@ def convert(povm, to_type, basis, ideal_povm=None, flatten_structure=False):
        are separately converted, leaving the original POVM's structure
        unchanged.  When `True`, composed and embedded operations are "flattened"
        into a single POVM of the requested `to_type`.
+
+    cptp_penalty : float, optional (default 1e-7)
+            Converting SPAM operations to error generator types includes extra degrees of gauge freedom (dumb gauge).


Let's not eternalize this naming choice in the documentation. If there are other places this was introduced take this comment as a blanket request to change those. Could you switch this to something like 'trivial gauge', or something else? Here is a candidate for new docstring:

Converting SPAM operations to an error generator representation may introduce trivial gauge degrees of freedom. These gauge degrees of freedom are called trivial because they quite literally do not change the dense representation (i.e. Hilbert-Schmidt vectors) at all. Despite being trivial, error generators along this trivial gauge orbit may be non-CP, so this cptp penalty is used to favor channels within this gauge orbit which are CPTP.

I'll also note that strictly speaking this is only a CP penalty, as TP is enforced by construction when using elementary error generators, but that is mostly a pedantic comment. You can leave the name alone.

coreyostrove · 2025-02-23T18:57:12Z

pygsti/modelmembers/povms/__init__.py

                    base_povm = UnconstrainedPOVM(base_items, povm.evotype, povm.state_space)

                proj_basis = 'PP' if povm.state_space.is_entirely_qubits else basis
                errorgen = _LindbladErrorgen.from_error_generator(povm.state_space.dim, lndtype, proj_basis,
                                                                  basis, truncate=True, evotype=povm.evotype)
+                #if to_type == 'GLND' and isinstance(povm, destination_types.get('full TP', NoneType)):


This commented out line looks like it can be safely deleted.

coreyostrove · 2025-02-23T19:03:07Z

pygsti/modelmembers/povms/__init__.py

+                #This has been called "the dumb gauge", and this function is meant to avoid it
+                def calc_physical_subspace(dense_ideal_povm, epsilon = 1e-9):
+
+                        errgen = _LindbladErrorgen.from_error_generator(povm.state_space.dim, parameterization=to_type)


Unless this is just a visual artifact on github it looks like you've double indented this function block. Could you fix that?

coreyostrove · 2025-02-23T19:09:36Z

pygsti/modelmembers/povms/__init__.py

+                soln = _spo.minimize(_objfn, _np.zeros(len(phys_directions), 'd'), method="Nelder-Mead", options={},
+                                        tol=1e-13)  # , callback=callback)
+                #print("DEBUG: opt done: ",soln.success, soln.fun, soln.x)  # REMOVE
+                if not soln.success and soln.fun > 1e-6:  # not "or" because success is often not set correctly


Remove the two REMOVE lines.

coreyostrove · 2025-02-23T20:23:04Z

pygsti/modelmembers/states/__init__.py

+                        errorgen.from_vector(L_vec)
+                        proc_matrix = _spl.expm(errorgen.to_dense())
+                        #basis of to_dense() CHECK if always pp
+                        dense_basis = Basis.cast('pp', (2**num_qubits)**2)


This shouldn't assume a hardcoded pauli-product basis. I don't think the error generator modelmember itself has a basis attribute to grab, but it should have a reference to a parent model which would have this available. Can you update this to grab the appropriate basis? (And if you were doing something similar for the POVMs and I missed it add the change there too.)

coreyostrove · 2025-02-23T20:24:03Z

pygsti/tools/optools.py

@@ -306,9 +306,6 @@ def diamonddist(a, b, mx_basis='pp', return_x=False):

    # currently cvxpy is only needed for this function, so don't import until here
    import cvxpy as _cvxpy
-    old_cvxpy = bool(tuple(map(int, _cvxpy.__version__.split('.'))) < (1, 0))
-    if old_cvxpy:
-        raise RuntimeError('CVXPY 0.4 is no longer supported. Please upgrade to CVXPY 1.0 or higher.')



Is this the official @rileyjmurray endorsed fix for this? Do we no longer need this check?

juangmendoza19 added 6 commits July 16, 2024 14:59

povm convert fixed for fullTP->GLND, missing CPTPLND considerations

627ec20

full TP -> GLND reparam now avoids dumb-gauge, minimizes 2norm

484fe3e

bypass our changes if reparam is not full TP -> GLND

6365db8

removed magic numbers, adapted for arbitrary number of qubits

246623e

removed temp files

d4fd818

removed magic numbers from povm convert, and added a check if there i…

167d742

…s valid GLND representation for povm

juangmendoza19 requested review from rileyjmurray and a team as code owners August 23, 2024 18:12

juangmendoza19 requested a review from sserita August 23, 2024 18:12

juangmendoza19 added 2 commits September 4, 2024 09:22

commented povm file

c6ec19f

commented state file

2bf56d6

sserita reviewed Sep 19, 2024

View reviewed changes

juangmendoza19 added 11 commits September 26, 2024 11:58

started implementing CPTPLND compatibility with set_all_parameterization

0ec92dd

added support for converting to and from many other models. Tested th…

b769193

…e subset of models GLND, CPTPLND, H+S, S, full TP all to all when converting models with no noise. There is currently no support for converting to the same type for many of these

made cptp_penalty an optional argument, made it 1e-7 accross all inst…

e8232b3

…ances

Merge remote-tracking branch 'origin/develop' into bugfix-reparameterize

5425f61

using proper warning library, started unit tests

d70fe7d

unit test for process matrices in reparameterization added

51d75a5

fixed assert syntax

d39ea28

fixed small bug in errgen parameter counting, now forward sim tests a…

32970ff

…re failing for some reason

fixed warning bug in povm convert code

6786b14

added proper criteria to test parameterization change

933425b

added comment to remember to add more tests in the future

a737603

juangmendoza19 commented Feb 20, 2025

View reviewed changes

juangmendoza19 added 3 commits February 21, 2025 11:52

added param counting back to unit test

16c7259

removed incorrect formula use for errgen model unit test

60e2673

Merge branch 'develop' into bugfix-reparameterize

abaff72

Catching up this branch with develop. Specifically I am trying to see if this fixes some unit tests that are currently failing.

juangmendoza19 requested review from kmrudin and a team as code owners February 21, 2025 19:15

juangmendoza19 added 2 commits February 21, 2025 13:54

removed check for old cvxpy

ad4b6ad

added unit tests for parameterization changes starting from a GLND model

7825c1b

juangmendoza19 changed the base branch from develop to master February 23, 2025 04:11

juangmendoza19 changed the base branch from master to develop February 23, 2025 04:11

juangmendoza19 requested a review from sserita February 23, 2025 04:23

coreyostrove requested changes Feb 23, 2025

View reviewed changes

implemented Corey's requests for the PR

18b9294

juangmendoza19 requested a review from coreyostrove February 24, 2025 19:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bugfix reparameterize #480

Bugfix reparameterize #480

juangmendoza19 commented Aug 23, 2024

sserita left a comment

juangmendoza19 Feb 20, 2025 •

edited

Loading

coreyostrove Feb 23, 2025

juangmendoza19 Feb 20, 2025

juangmendoza19 Feb 20, 2025

juangmendoza19 Feb 20, 2025

juangmendoza19 Feb 20, 2025

juangmendoza19 Feb 20, 2025

juangmendoza19 Feb 20, 2025

juangmendoza19 commented Feb 22, 2025

coreyostrove commented Feb 23, 2025

coreyostrove left a comment •

edited

Loading

coreyostrove Feb 23, 2025

coreyostrove Feb 23, 2025 •

edited

Loading

coreyostrove Feb 23, 2025

coreyostrove Feb 23, 2025

coreyostrove Feb 23, 2025

coreyostrove Feb 23, 2025

coreyostrove Feb 23, 2025

		TODO: 2 qubit labels returning None when label does exist in self.labels.
		'(support, left_support) not in self._offsets[eetype]' returning True incorrectly

Bugfix reparameterize #480

Are you sure you want to change the base?

Bugfix reparameterize #480

Conversation

juangmendoza19 commented Aug 23, 2024

sserita left a comment

Choose a reason for hiding this comment

juangmendoza19 Feb 20, 2025 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juangmendoza19 commented Feb 22, 2025

coreyostrove commented Feb 23, 2025

coreyostrove left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coreyostrove Feb 23, 2025 • 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

juangmendoza19 Feb 20, 2025 •

edited

Loading

coreyostrove left a comment •

edited

Loading

coreyostrove Feb 23, 2025 •

edited

Loading