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

Missing tests for gen_pauli_x and gen_pauli_z #485

Open
purva-thakre opened this issue Feb 23, 2024 · 6 comments · May be fixed by #544
Open

Missing tests for gen_pauli_x and gen_pauli_z #485

purva-thakre opened this issue Feb 23, 2024 · 6 comments · May be fixed by #544
Labels
good first issue Good for newcomers
Milestone

Comments

@purva-thakre
Copy link
Collaborator

purva-thakre commented Feb 23, 2024

Related to #478

Tests for these functions are missing from toqito/matrices/tests

Tests in matrices/tests Defined functions in matrices/
image image

https://github.com/vprusso/toqito/tree/eb5820721884dae5b9fdaad68913b2d96785167d/toqito/matrices/tests

Interestingly enough, both these functions are not flagged as uncovered by pytest.

image

This issue is to find where the tests for these functions are or add files to test them. If it's the latter, need to figure out why pytest is not flagging these.

@purva-thakre
Copy link
Collaborator Author

purva-thakre commented Feb 23, 2024

@vprusso Looking at the commit history for both functions, I don't think the tests for these functions were added. Wanted to double-check with you as to what might have happened. Maybe the tests got lost in other modules?

@vprusso vprusso changed the title MIssing tests for shift and clock Missing tests for shift and clock Feb 23, 2024
@purva-thakre
Copy link
Collaborator Author

purva-thakre commented Feb 23, 2024

Looking through the old commits, I don't see the tests in the previous tests directory.

We changed the directory in #257 and if I look at another PR #255 merged before the restructured directory, the tests don't exist where we expect them to be.

https://github.com/vprusso/toqito/tree/63c86059f06f994e23be93f3a4626c0b1717b464/tests/test_matrices

Maybe this is also a pytest issue as it does not flag the uncovered lines in addition to missing tests.

@purva-thakre
Copy link
Collaborator Author

purva-thakre commented Feb 23, 2024

Interestingly, if I look at the full coverage report, pytest thinks there are tests for these functions. I don't know what BrPart is though in the screenshot below.

image
image

@purva-thakre
Copy link
Collaborator Author

purva-thakre commented Feb 23, 2024

Maybe pytest thinks the lines below cover the expected output of other functions.

gen_pauli_x = shift(dim)
gen_pauli_z = clock(dim)

@vprusso
Copy link
Owner

vprusso commented Feb 24, 2024

Hmm, so I think these are covered because there is a test for gen_pauli which calls both of these functions. As there isn't any branching in either, testing them via that one function should be all that is required to have them covered from that.

But yes, having a few specific tests for them explicitly might be a good idea, just to be more thorough and explicit.

@purva-thakre purva-thakre changed the title Missing tests for shift and clock Missing tests for gen_pauli_x and gen_pauli_z Feb 24, 2024
@purva-thakre
Copy link
Collaborator Author

purva-thakre commented Feb 24, 2024

To close this issue after #478 is merged, in the folder toqito/matrices/tests, add a file labeled test_gen_pauli_x and another labeled test_gen_pauli_z for testing the functions via pytest.

List of all possible tests:

  1. Verify the shape of the matrices is what we expect
  2. Verify the expected output is what we expect for a bunch of different values for dim.

It might also be better to use the parameterized option in pytest to consolidate a bunch of identical tests.
https://docs.pytest.org/en/7.3.x/how-to/parametrize.html

Here's an example test in toqito where pytest verifies an array output.

@pytest.mark.parametrize("dim, p_param, partial, expected_result", [
# Dimension is 2 and p is equal to 1.
(2, 1, False, np.array([[1, 0], [0, 1]])),
# The `p` value is greater than the dimension `d`.
(2, 3, False, np.zeros((8, 8))),
# The dimension is 2.
(2, 2, False, np.array([[0, 0, 0, 0], [0, 0.5, -0.5, 0], [0, -0.5, 0.5, 0], [0, 0, 0, 0]])),
# The `dim` is 3, the `p` is 3, and `partial` is True.
(3, 3, True, anti_proj_3_3_partial)
])
def test_antisymmetric_projection(dim, p_param, partial, expected_result):
"""Test function works as expected for a valid input."""
proj = antisymmetric_projection(dim=dim, p_param=p_param, partial=partial).todense()
assert abs(proj - expected_result).all() <= 1E-3

@purva-thakre purva-thakre added the good first issue Good for newcomers label Feb 24, 2024
@purva-thakre purva-thakre added this to the v1.0.8 milestone Feb 24, 2024
@atomgardner atomgardner linked a pull request Apr 9, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants