antalszava commented 4 years ago

Context: One can include photon counting measurements within Program definitions. Doing so will result in obtaining PNR samples for the mode(s) on which the measurement acts. In general, samples obtained while running a Program on an Engine are contained in Result.samples. Such samples are in most cases contained in NumPy arrays having a default shape of (shots, modes).

We would like to allow getting counting statistics on samples that were obtained after running a Program that solely contained photon counting measurements. This addition is aimed at backends "fock" and "gaussian".

Description of the Change:

Adds the number_expectation_pnr and number_variance_pnr functions that take PNR samples
Adds user input validation checks
Defines the number_expectation and number_variance shorthands

Benefits: Counting statistics can be obtained for PNR samples.

Possible Drawbacks: N/A

Related GitHub Issues: N/A

TODO

[X] Further consideration on naming
[X] Add further unit tests for expval and var
[X] Adding examples
[X] Formatting

codecov[bot] commented 4 years ago

Codecov Report

Merging #399 into master will increase coverage by 0.06%. The diff coverage is 99.23%.

@@            Coverage Diff             @@
##           master     #399      +/-   ##
==========================================
+ Coverage   97.84%   97.90%   +0.06%     
==========================================
  Files          58       68      +10     
  Lines        6784     6982     +198     
==========================================
+ Hits         6638     6836     +198     
  Misses        146      146

Impacted Files	Coverage Δ
strawberryfields/api/result.py	`100.00% <ø> (ø)`
strawberryfields/apps/train/embed.py	`100.00% <ø> (ø)`
...awberryfields/backends/gaussianbackend/__init__.py	`100.00% <ø> (ø)`
strawberryfields/backends/gaussianbackend/ops.py	`100.00% <ø> (+2.70%)`	:arrow_up:
strawberryfields/circuitspecs/gaussian_unitary.py	`100.00% <ø> (ø)`
strawberryfields/circuitspecs/tensorflow.py	`100.00% <ø> (ø)`
strawberryfields/backends/fockbackend/ops.py	`94.04% <94.11%> (+2.04%)`	:arrow_up:
strawberryfields/ops.py	`98.84% <96.29%> (-0.19%)`	:arrow_down:
strawberryfields/backends/tfbackend/ops.py	`96.69% <97.88%> (-0.17%)`	:arrow_down:
strawberryfields/apps/data/feature.py	`98.36% <98.36%> (ø)`
... and 41 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 05e2771...83bce4e. Read the comment docs.

antalszava commented 4 years ago

Getting a linting issue on np.argwhere(samples == None). Using samples is None does not seem to fit here. (might be missing something)
Open for suggestions on the file organization and/or namespace & import details

antalszava commented 4 years ago

This should now be ready for review (samples now have no None values thanks to #398 :confetti_ball:).

Further changes:

Added a quadrature_expectation_homodyne, quadrature_variance_homodyne and an all_fock_probs_pnr function
post_processing.py has been moved to the new utils package

antalszava commented 4 years ago

Hi @josh146, thanks for the comments! :slightly_smiling_face:

Before moving to address the rest of the comments:

I feel convinced about having only two functions: sample_expectation and sample_variance. The original implementation was, however, adhering to what we've outlined when thinking about this change: having a signature of def number_expectation_pnr(photon_number_samples, modes=None): , where the function name refers to the operator, the statistics obtained (and to the type of sample to avoid ambiguity).

Before falling into the trap of modifying these functions back and forth, I'd just like to double-check if we'd for sure want to abandon that proposal and have only these two user-facing functions for expval and var.

This will have the advantage:

will not confuse users into thinking that different functions do different things (a premise that would be otherwise well assumed)
less complexity

On the other hand, it would need to be emphasized that all the following and only the following are supported by these functions:

PNR samples -> expval/var of NumberOperator
homodyne samples obtained measuring the X quadrature on modes -> expval/var of position
homodyne samples obtained by measuring the P quadrature on modes -> expval/var of momentum

(That is, for example, PNR samples cannot be used to obtain expval/var for position or momentum)

josh146 commented 4 years ago

Before falling into the trap of modifying these functions back and forth, I'd just like to double-check if we'd for sure want to abandon that proposal and have only these two user-facing functions for expval and var.

I think your clean implementation in this PR helped solidify that in my head 🙂 Originally, I was under the assumption that each function would have it's own logic. Since that is not the case, I think we can use this to better inform the design.

This will have the advantage:

will not confuse users into thinking that different functions do different things (a premise that would be otherwise well assumed)

less complexity

I agree fully!

(That is, for example, PNR samples cannot be used to obtain expval/var for position or momentum)

I think solely providing sample_expectations and sample_var actually makes this clearer from the user perspective. The functions no longer make any assumptions as to the type of measurement, nor the observable that is being measured. It is now up to the user to interpret the output --- the functions will be doing exactly as they say.

antalszava commented 4 years ago

Hi @josh146 @nquesada thanks for the comments! I'd hope to have answered them all, this should be ready for a next round

XanaduAI / strawberryfields

Counting statistics for post-processing samples #399

Codecov Report