cb norm #157

AmanieOxana · 2023-05-28T21:31:28Z

Description

Added cb norm function and test.
I tried the SDP on a unitary, the values are not very far apart, but not equal, help required. resolves #148

The cb trace norm is also known as the diamond norm, therefore renamed former diamond_norm.py to diamond_norm_distance.py . Btw np.linalg.norm is also applicable for matrices.

Todos

Notable points that this PR has either accomplished or will accomplish.

Check SDP implementation

Questions

Is it possible for hermitian preserving channel to use y0= y1 during the SDP algorithm?

Status

Help required to find out why unitary example I get 1.847765622725633 where I expect 1.414213562373095

…t the values are not very far apart, help required. The cb trace norm is also known as the diamond norm, therefore renamin former diamond norm to diamond norm distance. np.linalg.norm is also applicable for matrices.

vprusso · 2023-05-29T11:36:46Z

tests/test_channel_metrics/test_completely_bounded_trace_norm.py

@@ -0,0 +1,22 @@
+import numpy as np
+from toqito.channels import dephasing
+def test_diamond_norm_quantum_channel():


We should place in spacing between the functions and between the import statements and the functions. Refer to the other test files to see how this is done.

vprusso · 2023-05-29T11:37:34Z

tests/test_channel_metrics/test_completely_bounded_trace_norm.py

+def test_diamond_norm_quantum_channel():
+    """The diamond norm of a quantum channel is 1"""
+    phi = dephasing(1)
+    np.testing.assert_equal(np.isclose(diamond_norm(phi), 1, atol=1e-3), True)


Since this function will return 1 in the event that the channel is quantum, I wonder if we need the isclose statement.

vprusso · 2023-05-29T11:41:15Z

toqito/channel_metrics/completely_bounded_trace_norm.py

+    :return: The completely bounded trace norm of the channel
+    """
+
+    elif is_quantum_channel(phi):


An elif should follow an if, I'm not sure if we want to start this chain of elif statements without starting it with an if.

vprusso · 2023-05-29T11:41:49Z

toqito/channel_metrics/completely_bounded_trace_norm.py

+from toqito.state_metrics import trace_norm
+import cvxpy as cp
+
+def diamond_norm(phi: np.ndarray)-> float:


Should the function name here be completely_bounded_trace_norm?

vprusso · 2023-05-29T11:42:54Z

toqito/channel_metrics/diamond_norm_distance.py

@@ -3,7 +3,7 @@
 import numpy as np


-def diamond_norm(choi_1: np.ndarray, choi_2: np.ndarray) -> float:
+def diamond_norm_distance(choi_1: np.ndarray, choi_2: np.ndarray) -> float:


Since the diamond_norm function is already in the channel_metrics/ folder, it's already implied that this is a distance metric. I would opt to keep the name diamond_norm over diamond_norm_distance (same with the file name as well, etc.)

vprusso · 2023-05-29T11:44:14Z

toqito/channel_metrics/completely_bounded_spectral_norm.py

+"Compute the completely bounded spectral norm of a quantum channel"
+from toqito.channel_ops import dual_channel
+from toqito.channel_metrics import completely_bounded_trace_norm
+def cb_spectral_norm(phi):


To maintain convention with the rest of the project, I would opt to rename the function to completely_bounded_spectral_norm. It is a bit longer, but more explicit, and if the user wishes to condense the name on import, they can do that if they wish.

vprusso · 2023-05-29T11:45:41Z

toqito/channel_metrics/completely_bounded_trace_norm.py

+            constraints += [y1 >> 0]
+            A = cp.bmat([[y0, -phi], [-np.conj(phi).T, y1]])
+            constraints += [  A >> 0  ]
+            objective = cp.Minimize( cp.atoms.norm_inf(cp.partial_trace(y0, dims= (dim,dim), axis = 1))


I should really configure some linting pre-commit hooks to catch these things, but as a minor nitpick, dims=(dim,dim) instead of dims = (dim, dim).

vprusso · 2023-05-29T11:46:59Z

tests/test_channel_metrics/test_completely_bounded_trace_norm.py

+        phi = np.array([[1, 2, 3], [4, 5, 6]])
+        diamond_norm(phi)
+
+if __name__ == "__main__":


It seems odd to me that the test_completely_bounded_trace_norm file is testing the diamond norm functionality, right? We probably should also add explicit tests for the completely_bounded_spectral_norm as well.

I wanted to call completely_bounded_trace_norm diamond norm, as in the Wikipedia article, which is easy to confuse with the diamond norm distance functionality in toqito. But sticking to completely_bounded_trace_norm is maybe easier in implementation.

Ah, I see where the issue here might be:

Indeed, the completely bounded trace norm is equivalent to the diamond norm (you are completely correct)

The diamond_norm function as is present on toqito computes the SDP for the diamond norm.

My mistake in writing the title of this issue is that I should have specified that this is for the "completely bounded norm" (not just the "completely bounded trace norm").

There shouldn't be too much to change on your end though as the completely bounded norm of a channel is equal to the diamond norm of the dual map.

Does that make sense? Let me know if anything I've said above isn't clear, and I'll be happy to clarify. Thank you again!

According to sec. 4 the SDP for the distance between two channels is simpler than the SDP for the completely bounded trace norm of channels (sec. 3). Have you implemented the SDP of sec. 4 in diamond_norm.py @vprusso ?
I have not implemented SDP from sec.3, but the QETLAB SDP, which is also cited in Wikipedia. Yet, i have not seen a mathematical derivaton of this formulation.

Hi @AmanieOxana

Right, the diamond_norm.py file in toqito (should be) implementing the diamond norm as specified in this QETLAB file:
https://github.com/nathanieljohnston/QETLAB/blob/master/DiamondNorm.m

And, ideally, the completely_bounded_norm.py file in toqito for this feature would implement the same functionality as in this file on QETLAB:
https://github.com/nathanieljohnston/QETLAB/blob/master/CBNorm.m

Does that clear things up or have I made things more confusing?

Hi @vprusso,

diamond_norm.py in toqito computes the distance between two quantum channels in contrast two DiamondNorm of QETLAB which computes the diamond norm of a single channel. As stated here the SDP for the distance and for the single channel diamond norm are different in nature (comparing sec. 3 and 4). So I do not agree that

diamond_norm.py file in toqito (should be) implementing the diamond norm > as specified in this QETLAB file:

Therefore this pull request includes two files, one for the completely bounded trace norm (aka diamond norm) and for the completely bounded spectral norm (to what you refered as cb norm)

It is a new day and I have a cup of coffee now. I believe (somehow) I missed what you are saying here (which you were saying perfectly clearly before, I just missed it).

I agree with everything you've written above. Thank you for sticking with me and your patience :)

vprusso · 2023-05-29T11:47:59Z

Hi @AmanieOxana . Thank you for your contribution! Added some comments on your diff. Do let me know if you have any questions, and thank you again for making the effort to work ontoqito!

AmanieOxana · 2023-05-29T14:35:03Z

I wanted to reproduce the unitary channel test from here.

U = 1 / np.sqrt(2) * np.array([[1, 1], [-1, 1]])  
phi = np.array([[np.eye(2), np.eye(2)], [U, -U]])
phi = phi.reshape((4, 4))

But is_unitary(phi) gives False. Does the reshaping change the map? @vprusso

vprusso · 2023-05-29T22:28:36Z

I wanted to reproduce the unitary channel test from here.
U = 1 / np.sqrt(2) * np.array([[1, 1], [-1, 1]])  
phi = np.array([[np.eye(2), np.eye(2)], [U, -U]])
phi = phi.reshape((4, 4))
But is_unitary(phi) gives False. Does the reshaping change the map? @vprusso

Hmm, it might be the case that you are invoking the incorrect is_unitary function. For instance, there is an is_unitary that is applicable to matrices under matrix_props and then another is_unitary function that is applicable to quantum channels under channel_props. Are you invoking the correct one?

AmanieOxana · 2023-05-31T11:14:49Z

Hi @vprusso, I corrected the norm in the SDP and a factor 1/2, now the tests work.
Still open is the is_unitary case and potentially a hermitian_preserving case. I still miss a mathematical derivation of this SDP, to decide the latter.

vprusso · 2023-05-31T11:27:22Z

Hi @vprusso, I corrected the norm in the SDP and a factor 1/2, now the tests work. Still open is the is_unitary case and potentially a hermitian_preserving case. I still miss a mathematical derivation of this SDP, to decide the latter.

Thanks for the update, @AmanieOxana! Let me know if I can help, and thank you again for the update on this!

AmanieOxana · 2023-06-03T08:42:00Z

Hi @vprusso, I have found a reference for the SDP. The case where the cb trace norm is the spectral diameter, could hold for mixed unitaries, but I have no specifications for this theorem at hand, that I just found in the qetlab wiki, so I left it out. For the hermitian preserving case, the only simplification I see is that the choi matrix is hermitian, but this does not make the sdp significantly faster.
Would you revise the current version of the implementation? If you have ideas for further enhancements, I am happy to hear about them.

tests/test_channel_metrics/test_completely_bounded_trace_norm.py

Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com>

vprusso

Hi @AmanieOxana ,

Thanks for the ping. Overall, looks great! A few nit-pick comments, but excellent overall!

vprusso · 2023-06-04T14:39:44Z

toqito/channel_metrics/completely_bounded_trace_norm.py

+        objective = cp.Minimize(  cp.norm(cp.partial_trace(y0, dims=(dim,dim), axis = 1))
+                                 +  cp.norm(cp.partial_trace(y1, dims=(dim,dim), axis = 1)) )
+        problem = cp.Problem(objective, constraints)
+        problem.solve(eps=1e-10)


This keyword will fail if the solver selected is not SCS. The solver options are listed here:
https://www.cvxpy.org/tutorial/advanced/index.html#setting-solver-options

However, even if one selects the SCS solver and does the following:

problem.solve(solver="SCS", eps=1e-10)

we get a warning:

tests/test_channel_metrics/test_completely_bounded_trace_norm.py::test_cb_trace_norm_unitaries_channel /Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/cvxpy/problems/problem.py:1337: UserWarning: Solution may be inaccurate. Try another solver, adjusting the solver settings, or solve with verbose=True for more information. warnings.warn(

As the tolerance of 1e-10 is fairly aggressive. I would suggest ratcheting that back to something lower. I would suggest altering:

problem.solve()

toqito/channel_metrics/completely_bounded_trace_norm.py

vprusso · 2023-06-04T14:45:31Z

tests/test_channel_metrics/test_completely_bounded_trace_norm.py

+
+
+def test_cb_trace_norm_unitaries_channel():
+    """The diamond norm of phi = id- U id U* is the diameter of the smallest circle that contains the eigenvalues of U"""


Nitpick: (period at end of comment):

Suggested change

"""The diamond norm of phi = id- U id U* is the diameter of the smallest circle that contains the eigenvalues of U"""

"""The diamond norm of phi = id- U id U* is the diameter of the smallest circle that contains the eigenvalues of U."""

vprusso · 2023-06-04T14:45:58Z

tests/test_channel_metrics/test_completely_bounded_trace_norm.py

@@ -0,0 +1,33 @@
+"""Tests for completely_bounded_trace_norm"""


Nitpick (period at end of statement):

Suggested change

"""Tests for completely_bounded_trace_norm"""

"""Tests for completely_bounded_trace_norm."""

vprusso · 2023-06-04T14:46:33Z

tests/test_channel_metrics/test_completely_bounded_trace_norm.py

+
+
+def test_cb_trace_norm_quantum_channel():
+    """The diamond norm of a quantum channel is 1"""


Nitpick (period at end of statement):

Suggested change

"""The diamond norm of a quantum channel is 1"""

"""The diamond norm of a quantum channel is 1."""

vprusso · 2023-06-04T14:47:28Z

tests/test_channel_metrics/test_completely_bounded_trace_norm.py

+
+
+def test_cb_trace_norm_unitaries_channel():
+    """The diamond norm of phi = id- U id U* is the diameter of the smallest circle that contains the eigenvalues of U"""


Nitpick (period at end of statement)

Suggested change

"""The diamond norm of phi = id- U id U* is the diameter of the smallest circle that contains the eigenvalues of U"""

"""The diamond norm of phi = id- U id U* is the diameter of the smallest circle that contains the eigenvalues of U."""

vprusso · 2023-06-04T14:48:45Z

toqito/channel_metrics/completely_bounded_spectral_norm.py

@@ -0,0 +1,28 @@
+"""Compute the completely bounded spectral norm of a quantum channel"""


Nitpick (period at end of statement):

Suggested change

"""Compute the completely bounded spectral norm of a quantum channel"""

"""Compute the completely bounded spectral norm of a quantum channel."""

vprusso · 2023-06-04T14:49:09Z

toqito/channel_metrics/completely_bounded_spectral_norm.py

+
+def completely_bounded_spectral_norm(phi: np.ndarray) -> float:
+    r"""
+    Compute the completely bounded spectral norm of a quantum channel


Nitpick (period at end of statement):

Suggested change

Compute the completely bounded spectral norm of a quantum channel

Compute the completely bounded spectral norm of a quantum channel.

vprusso · 2023-06-04T14:50:19Z

toqito/channel_metrics/completely_bounded_trace_norm.py

+        v = apply_channel(np.eye(dim_Ly), dual_channel(phi))
+        return trace_norm(v)
+
+    else:


Technically, you can remove this else: statement, as the above conditions will either raise or return.

vprusso · 2023-06-04T14:51:06Z

toqito/channel_metrics/completely_bounded_trace_norm.py

@@ -0,0 +1,65 @@
+"""Compute the completely bounded trace norm of a quantum channel"""


Nitpick (period at end of statement):

Suggested change

"""Compute the completely bounded trace norm of a quantum channel"""

"""Compute the completely bounded trace norm of a quantum channel."""

vprusso

Hi @AmanieOxana ,

Thanks for the ping. Overall, looks great! A few nit-pick comments, but excellent overall!

Co-authored-by: Vincent Russo <vincentrusso1@gmail.com>

AmanieOxana · 2023-06-04T20:31:00Z

Thanks for the review @vprusso. I have changed accordingly.

vprusso

Left a few minor nit-pick comments (I promise these are the last :) )

Approved and good to go once those are in! Great job, @AmanieOxana !

vprusso · 2023-06-04T23:47:29Z

tests/test_channel_metrics/test_completely_bounded_spectral_norm.py

+"""Tests for completely_bounded_spectral_norm."""
+import numpy as np
+
+from toqito.channel_metrics.completely_bounded_spectral_norm import completely_bounded_spectral_norm


We should make sure that in channel_metrics/__init__.py that we add the following lines:

from toqito.channel_metrics.completely_bounded_spectral_norm import completely_bounded_spectral_norm from toqito.channel_metrics.completely_bounded_trace_norm import completely_bounded_trace_norm

Then, the import line can be made somewhat more succinct as:

Suggested change

from toqito.channel_metrics.completely_bounded_spectral_norm import completely_bounded_spectral_norm

from toqito.channel_metrics import (

completely_bounded_spectral_norm,

completely_bounded_trace_norm,

)

vprusso · 2023-06-04T23:48:25Z

tests/test_channel_metrics/test_completely_bounded_trace_norm.py

+import numpy as np
+
+from toqito.channels.dephasing import dephasing
+from toqito.channel_metrics.completely_bounded_trace_norm import completely_bounded_trace_norm


Assuming you made the __init__.py alteration, you can change this line as well to:

Suggested change

from toqito.channel_metrics.completely_bounded_trace_norm import completely_bounded_trace_norm

from toqito.channel_metrics import completely_bounded_trace_norm

@vprusso I made this alteration, but this import seems not to work, as not the function but a module is imported. Should I change it to the prior version, or is sth wrong with the init?

Looks like the __init__.py is incorrect. For clarity, the __init__.py will look like this:

from toqito.channel_metrics.channel_fidelity import channel_fidelity from toqito.channel_metrics.diamond_norm import diamond_norm from toqito.channel_metrics.completely_bounded_spectral_norm import completely_bounded_spectral_norm from toqito.channel_metrics.completely_bounded_trace_norm import completely_bounded_trace_norm

Then, if you want to refer to these functions in another file, you can write the more condensed:

from toqito.channel_metrics import X

instead of the longer statement

from toqito.channel_metrics.X import X

Does that make sense?

changed it accordingly @vprusso , I though the init could be made also more concise, but should be fine

vprusso · 2023-06-04T23:48:52Z

tests/test_channel_metrics/test_completely_bounded_trace_norm.py

+
+from toqito.channels.dephasing import dephasing
+from toqito.channel_metrics.completely_bounded_trace_norm import completely_bounded_trace_norm
+from toqito.channel_ops.kraus_to_choi import kraus_to_choi


Likewise:

Suggested change

from toqito.channel_ops.kraus_to_choi import kraus_to_choi

from toqito.channel_ops import kraus_to_choi

vprusso · 2023-06-05T00:00:42Z

toqito/channel_metrics/completely_bounded_trace_norm.py

+    :param phi: superoperator as choi matrix
+    :return: The completely bounded trace norm of the channel
+    """
+    dim_Lx, dim_Ly = phi.shape


Nitpick: Having capital letters in variable names goes against PEP-8 (link)

Perhaps we can rename dim_Lx and dim_Ly to dim_lx and dim_ly, respectively? Or do you disagree?

vprusso · 2023-06-05T00:01:27Z

toqito/channel_metrics/completely_bounded_trace_norm.py

+    """
+    dim_Lx, dim_Ly = phi.shape
+
+    if dim_Lx != dim_Ly:


The way you are doing this here is completely fine, although toqito does have a matrix_props.is_square function that can also be invoked. Your call as to whether or not you'd like to use that here or not.

vprusso · 2023-06-05T00:02:09Z

toqito/channel_metrics/completely_bounded_trace_norm.py

+    constraints += [y1 == y1.H]
+    constraints += [y1 >> 0]
+
+    A = cp.bmat([[y0, -phi], [-phi.conj().T, y1]])


One more pedantic PEP-8 suggestion of changing A to a_var (or something like that).

vprusso

Left a few minor nit-pick comments (I promise these are the last :) )

Approved and good to go once those are in! Great job, @AmanieOxana !

vprusso

Left a few minor nit-pick comments (I promise these are the last :) )

Approved and good to go once those are in! Great job, @AmanieOxana !

vprusso · 2023-06-05T11:06:18Z

Thanks for the quick update, @AmanieOxana . Let me know if I can provide any further help and feel free to let me know when you think this is ready to merge!

AmanieOxana · 2023-06-05T12:22:01Z

Sure, unless you have no further ideas for simplifications when the channel is unitary or hermitian @vprusso, I think it is ready.

vprusso · 2023-06-05T13:15:31Z

Sure, unless you have no further ideas for simplifications when the channel is unitary or hermitian @vprusso, I think it is ready.

Excellent! Well, in that case, I will go ahead and put this through! Thank you again, @AmanieOxana, for your contribution to the toqito project and I have that this was a fun experience for you! It was a privilege on my side, and I hope to see you around here again!

AmanieOxana added 4 commits May 28, 2023 23:21

Added is_unitary cases for diamond norm

37bf037

Added is_herm_perserving cases for diamond norm

5bda675

Correct typo if statements

9109c0b

vprusso reviewed May 29, 2023

View reviewed changes

AmanieOxana added 2 commits May 29, 2023 15:31

Enhanced readability

ea2ed6d

Add test for cb spectral norm

b30d5de

AmanieOxana added 2 commits May 31, 2023 12:55

used correct norm

57310bf

had to add divide by 1/2

31a75cd

AmanieOxana added 2 commits June 1, 2023 21:50

removed is_unitary case

a6b010e

correct reference

c39ac22

georgios-ts reviewed Jun 3, 2023

View reviewed changes

tests/test_channel_metrics/test_completely_bounded_trace_norm.py Outdated Show resolved Hide resolved

AmanieOxana and others added 2 commits June 3, 2023 19:00

Update tests/test_channel_metrics/test_completely_bounded_trace_norm.py

dcf0873

Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com>

correct kraus_to_choi

dca4cdd

vprusso reviewed Jun 4, 2023

View reviewed changes

AmanieOxana and others added 3 commits June 4, 2023 22:16

Update toqito/channel_metrics/completely_bounded_trace_norm.py

ff5e9d2

Co-authored-by: Vincent Russo <vincentrusso1@gmail.com>

remove else and add periods

0edecc6

period

a1c3a8d

vprusso approved these changes Jun 5, 2023

View reviewed changes

vprusso mentioned this pull request Jun 5, 2023

Move Trace Norm #172

Merged

1 task

AmanieOxana added 2 commits June 5, 2023 08:47

alterate imports, yet for cbnorms import err

a89c464

correct import

2d5f972

vprusso merged commit 12934c0 into vprusso:master Jun 5, 2023



		def test_cb_trace_norm_unitaries_channel():
		"""The diamond norm of phi = id- U id U* is the diameter of the smallest circle that contains the eigenvalues of U"""

		@@ -0,0 +1,33 @@
		"""Tests for completely_bounded_trace_norm"""



		def test_cb_trace_norm_quantum_channel():
		"""The diamond norm of a quantum channel is 1"""

		@@ -0,0 +1,28 @@
		"""Compute the completely bounded spectral norm of a quantum channel"""

	Compute the completely bounded spectral norm of a quantum channel
	Compute the completely bounded spectral norm of a quantum channel.

		@@ -0,0 +1,65 @@
		"""Compute the completely bounded trace norm of a quantum channel"""

-from toqito.channel_metrics.completely_bounded_spectral_norm import completely_bounded_spectral_norm
+from toqito.channel_metrics import (
+    completely_bounded_spectral_norm,
+    completely_bounded_trace_norm,
+)

	from toqito.channel_metrics.completely_bounded_trace_norm import completely_bounded_trace_norm
	from toqito.channel_metrics import completely_bounded_trace_norm

	from toqito.channel_ops.kraus_to_choi import kraus_to_choi
	from toqito.channel_ops import kraus_to_choi

cb norm #157

cb norm #157

Conversation

AmanieOxana commented May 28, 2023 • edited Loading

Description

Todos

Questions

Status

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

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

AmanieOxana May 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vprusso commented May 29, 2023

AmanieOxana commented May 29, 2023 • edited Loading

vprusso commented May 29, 2023 • edited Loading

AmanieOxana commented May 31, 2023

vprusso commented May 31, 2023

AmanieOxana commented Jun 3, 2023

vprusso 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vprusso left a comment

Choose a reason for hiding this comment

AmanieOxana commented Jun 4, 2023

vprusso 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vprusso left a comment

Choose a reason for hiding this comment

vprusso left a comment

Choose a reason for hiding this comment

vprusso commented Jun 5, 2023

AmanieOxana commented Jun 5, 2023

vprusso commented Jun 5, 2023

AmanieOxana commented May 28, 2023 •

edited

Loading

AmanieOxana May 30, 2023 •

edited

Loading

AmanieOxana commented May 29, 2023 •

edited

Loading

vprusso commented May 29, 2023 •

edited

Loading