Type checking for `channel_metrics`

purva-thakre commented 1 month ago

Description

Related to #299

We use --explicit-package-bases here to disable mypy errors due to 2 files of the same name existing in different modules.

Changes

Right now ~/toqito$ mypy toqito/channel_metrics --explicit-package-bases is failing due to the following. These are mostly due to incompatibilities with type annotations in cvxpy compared to how we use them.

toqito/channel_metrics/diamond_norm.py:102: error: Argument 1 to "Parameter" has incompatible type "list[int]"; expected "int | tuple[int, ...]"  [arg-type]
toqito/channel_metrics/completely_bounded_trace_norm.py:67: error: Argument "dims" to "partial_trace" has incompatible type "tuple[int, int]"; expected "tuple[int]"  [arg-type]
toqito/channel_metrics/completely_bounded_spectral_norm.py:35: error: Module not callable  [operator]

Will need to refactor portions of:

diamon_norm.py:

https://github.com/vprusso/toqito/blob/1122a6e6aeb4fe8c41dbafded6fccbbcb7a40453/toqito/channel_metrics/diamond_norm.py#L102

Here, [dim_squared, dim_Squared] is of type List[int] whereas cxpy.Parameter expects shape: int | tuple[int, ...] https://www.cvxpy.org/api_reference/cvxpy.expressions.html#parameter

completely_bounded_trace_norm.py

Same as the previous item, we use an incompatible type compared to what's expected by cvpy type annotation. It expects tuple[int] but we use tuple[int. int].

https://github.com/vprusso/toqito/blob/1122a6e6aeb4fe8c41dbafded6fccbbcb7a40453/toqito/channel_metrics/completely_bounded_trace_norm.py#L67

https://www.cvxpy.org/api_reference/cvxpy.atoms.affine.html#partial-trace

completely_bounded_spectral_norm.py

For some reason, completely_bounded_trace_norm or dual_channel shows up as a module not callable.

https://github.com/vprusso/toqito/blob/1122a6e6aeb4fe8c41dbafded6fccbbcb7a40453/toqito/channel_metrics/completely_bounded_spectral_norm.py#L35

If I specify the full path for completely_bounded_trace_norm as completely_bounded_trace_norm.completely_bounded_trace_norm, mypy fails with the following error:

toqito/channel_metrics/completely_bounded_spectral_norm.py:35: error: Argument 1 to "completely_bounded_trace_norm" has incompatible type "ndarray[Any, Any] | list[list[ndarray[Any, Any]]]"; expected "ndarray[Any, Any]"  [arg-type]

Checklist

Before marking your PR ready for review, make sure you checked the following locally. If this is your first PR, you might be notified of some workflow failures after a maintainer has approved the workflow jobs to be run on your PR.

Additional information is available in the documentation.

[ ] Use ruff for errors related to code style and formatting.
[ ] Verify all previous and newly added unit tests pass in pytest.
[ ] Check the documentation build does not lead to any failures. Sphinx build can be checked locally for any failures related to your PR
[ ] Use linkcheck to check for broken links in the documentation
[ ] Use doctest to verify the examples in the function docstrings work as expected.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.1%. Comparing base (1122a6e) to head (a7381f9).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #623 +/- ## ====================================== Coverage 98.1% 98.1% ====================================== Files 162 162 Lines 3112 3115 +3 Branches 759 759 ====================================== + Hits 3054 3057 +3 Misses 37 37 Partials 21 21 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

purva-thakre commented 1 month ago

I think the type annotation for dims argument in partial_trace is wrong.

It is dims: Tuple[int] when it should be dims: Tuple[int, ...]. We see the latter being used by cvxpy.partial_transpose.

https://www.cvxpy.org/api_reference/cvxpy.atoms.affine.html#partial-transpose

To Do: Understand if I need to raise this in the cvxpy repo.

vprusso / toqito