Adding probability tests #368
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## internal_modes #368 +/- ##
=================================================
Coverage ? 94.07%
=================================================
Files ? 32
Lines ? 2262
Branches ? 0
=================================================
Hits ? 2128
Misses ? 134
Partials ? 0 Continue to review full report in Codecov by Sentry.
|
| @@ -1419,7 +1432,10 @@ def test_density_matrix(cutoff): | |||
| rho2 = density_matrix_single_mode(cov, N, cutoff=cutoff) | |||
| rho2_norm = rho2 / np.trace(rho2).real | |||
|
|
|||
| # probs = probabilities_single_mode(cov, N, cutoff=cutoff, normalize=True) | |||
There was a problem hiding this comment.
@DavidSPhillips this is a waaay too long test..
There was a problem hiding this comment.
Does it take ages in the internal_modes branch?
There was a problem hiding this comment.
well, it takes ages on my computer, that is why I decided against adding the new functions in this test
There was a problem hiding this comment.
What's too long? The whole test or the addition of the commented out probabilities bit? The new bit doesn't take much longer for my laptop at all (after correcting for the PNR on mode 0). The whole test takes about 2.5 minutes.
There was a problem hiding this comment.
The test. 2.5 minutes is too long.
There was a problem hiding this comment.
But the test is still included so it will still take 2.5 minutes. Adding the commented out lines (but not commented out) wouldn't increase the time much.
| return netsum | ||
|
|
||
|
|
||
| def probabilities_single_mode(cov, pattern, normalize=False, LO_overlap=None, cutoff=13, hbar=2): |
There was a problem hiding this comment.
What is this function supposed to be? In the docstring it says density matrix but should it say probability pattern? Also I don't really understand why there's an option to project onto LO for this, given we assume the PNRs are bucket detectors, it shouldn't affect how we split the internal modes up, no?
There was a problem hiding this comment.
I have rewritten the doc string, hopefully this clarifies the meaning.
There was a problem hiding this comment.
I understand, it's essentially the pnr distribution of the 'principal mode' (or mode of our choosing). Still, I feel it might be more appropriate to call it something like dm_diag_single_mode (or something to that effect) to avoid confusion with pnr_prob, but ultimately I don't feel that strongly about it.
There was a problem hiding this comment.
well, PNR prob does something similar, but the signature of the function is different.
|
After the docstring change all tests pass? It was |
The tests from this branch are still failing on wsl my laptop (with numba=0.55.1), but I re-run the jobs here and it's still passing. |
| from ..quantum import Qmat | ||
| from .._hafnian import find_kept_edges, nb_binom, f_from_powertrace, nb_ix | ||
| from ..quantum import Qmat, Amat | ||
| from thewalrus.symplectic import passive_transformation |
There was a problem hiding this comment.
Why not a relative import like the other imports?
There was a problem hiding this comment.
does that make a difference?
Co-authored-by: rachelchadwick <[email protected]>
…rus into adding_probability_tests
|
@rachelchadwick is this good to be merged? |
Adds non-batched internal modes calculation of probabilities.