search

XanaduAI / strawberryfields

Strawberry Fields is a full-stack Python library for designing, simulating, and optimizing continuous variable (CV) quantum optical circuits.
https://strawberryfields.ai
Apache License 2.0
745 stars 187 forks source link

Switch measurements kwargs to args in `io.to_blackbird` #622

Closed thisac closed 3 years ago

thisac commented 3 years ago

Context: Measurement arguments are always compiled into Blackbird keyword arguments when using io.to_blackbird, while all other gate arguments are being stored as non-keyword arguments. This causes some issues when attempting to match templates with parameter values (specifically for QKD).

Description of the Change: Switch measurement arguments from being compiled into keyword arguments in Blackbird to non-keyword arguments, to conform with the rest of the gate arguments.

Benefits: Matching parameter values with measurement arguments works the same as when matching with gate arguments.

Possible Drawbacks: If someone somewhere uses the fact that measurement arguments always are stored as keyword arguments in the resulting Blackbird program, this will no longer work (since the parameters will be found as a non-keyword argument instead).

Related GitHub Issues: None

codecov[bot] commented 3 years ago

Codecov Report

Merging #622 (0bd7d94) into master (a5e5f44) will decrease coverage by 0.02%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #622      +/-   ##
==========================================
- Coverage   98.56%   98.54%   -0.03%     
==========================================
  Files          77       77              
  Lines        8938     8938              
==========================================
- Hits         8810     8808       -2     
- Misses        128      130       +2
Impacted Files Coverage Δ
strawberryfields/io.py 95.87% <100.00%> (-1.04%) :arrow_down:

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 a5e5f44...0bd7d94. Read the comment docs.

thisac commented 3 years ago

I think this is fine to be merged. I'm not sure why codecov is complaining about less coverage (nothing has really been removed nor added), but perhaps we could just override it (@josh146?).

Otherwise I just need an approval, and then I can merge.