Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move references in docstrings #228

Merged
merged 43 commits into from
Nov 25, 2023
Merged

Move references in docstrings #228

merged 43 commits into from
Nov 25, 2023

Conversation

purva-thakre
Copy link
Collaborator

Description

Fixes #191

Todos

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

  • TODO 1

Questions

  • Question1

Status

  • Ready to go

Copy link

codecov bot commented Nov 10, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (7e419bf) 98.1% compared to head (2d41cf2) 98.1%.

Files Patch % Lines
toqito/nonlocal_games/nonlocal_game.py 80.0% 0 Missing and 1 partial ⚠️
...te_opt/tests/test_symmetric_extension_hierarchy.py 50.0% 1 Missing ⚠️
toqito/state_props/is_separable.py 0.0% 1 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##           master    #228   +/-   ##
======================================
  Coverage    98.1%   98.1%           
======================================
  Files         295     295           
  Lines        6929    6929           
  Branches      773     773           
======================================
  Hits         6798    6798           
  Misses         90      90           
  Partials       41      41           

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

docs/conf.py Outdated Show resolved Hide resolved
@purva-thakre
Copy link
Collaborator Author

@vprusso Are you fine with how QETLAB is cited throughout the references? Thought I should double-check before I do the same for others.

image

@vprusso
Copy link
Owner

vprusso commented Nov 19, 2023

@vprusso Are you fine with how QETLAB is cited throughout the references? Thought I should double-check before I do the same for others.

image

Yep, I think that seems fine to me!

@purva-thakre
Copy link
Collaborator Author

Reopened this PR. The issue was with pybtex not being able to parse some URLs as URLs. For now, all the URLs that caused issues are strings. Will create a new issue for this.

@purva-thakre
Copy link
Collaborator Author

@vprusso This is ready for a review after resolving the conflicts.

Copy link
Owner

@vprusso vprusso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments, but overall, LGTM modulo those comments!

@@ -6,20 +6,14 @@


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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the citations here be on the same line?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line length > 120 if I have the citations on the same line.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, gotcha. Makes sense!

@@ -12,23 +12,15 @@ def completely_bounded_trace_norm(phi: np.ndarray) -> float:
Find the completely bounded trace norm of a quantum channel.

Also known as the completely bounded diamond norm of a quantum
channel [WatCNorm18]_. The algorithm in p.11 of [WatSDP09] with
implementation in QETLAB [JohQET] is used.
channel (Section 3.3.2 of :cite:`watrous_2018`). The algorithm in p.11 of :cite:`watrous2012simpler` with
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use a consistent bibtex key naming convention? For instance we have here watrous_2018 but then elsewhere we might have something like Watrous_2009_semidefinite. Perhaps we can agree upon on convention for consistency?

Something like {author-last-name}{year}{first-word-of-paper} so for instance, Watrous_2009_semidefinite would be watrous2009semidefinite?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I created a new issue for this #276 . I was lazy and mostly used what was provided in the bib info by arxiv because the changes are becoming too big to keep track of in this PR.

toqito/channel_metrics/diamond_norm.py Outdated Show resolved Hide resolved
toqito/channel_ops/choi_to_kraus.py Outdated Show resolved Hide resolved
toqito/channel_ops/choi_to_kraus.py Outdated Show resolved Hide resolved
toqito/helper/npa_hierarchy.py Outdated Show resolved Hide resolved
toqito/matrices/gen_pauli.py Outdated Show resolved Hide resolved
toqito/matrix_ops/vectors_to_gram_matrix.py Outdated Show resolved Hide resolved
toqito/state_props/tests/test_entanglement_of_formation.py Outdated Show resolved Hide resolved
toqito/states/isotropic.py Outdated Show resolved Hide resolved
@@ -1,15 +1,16 @@
#File for all article, arxiv and other web references
# To add a key, Lastname_year_abbreviatedtitle/firstletteroftitle
Copy link
Collaborator Author

@purva-thakre purva-thakre Nov 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To add a key, Lastname_year_abbreviatedtitle/firstletteroftitle

@vprusso Would this work for the sake of consistency?

I did see your comment on this #228 (comment) but I would prefer to separate last name, year and first one/two words of title by underscore.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could align with how Google Scholar does bibtex keys, which is mostly what you have there. That is {lastname}{year}{abbreviatedtitle} Any opposition to that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I am going to create an issue for using Google scholar style for bibtex keys. I'll get to making these changes then.

@purva-thakre purva-thakre merged commit 70c28cf into master Nov 25, 2023
3 checks passed
@purva-thakre purva-thakre deleted the move_docstring_ref branch November 25, 2023 15:54
@purva-thakre purva-thakre mentioned this pull request Dec 30, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use one reference file for docstrings
2 participants