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

747 stars 186 forks source link

Cleanup TF todo items #567

Closed Aaron-Robertson closed 3 years ago

Aaron-Robertson commented 3 years ago

Context: Items marked todo during initial implementation of the tfbackend are ready for cleanup.

Description of the Change: Resolves cleanup items in tfbackend based on updated version of tensorflow.

Benefits: Resolves todo items to reduce line count, clarify function similarity across backend ops, and clean the approach to tensorflow.tensordot based on TF version >2.0.

Possible Drawbacks: N/A

Related GitHub Issues: N/A

codecov[bot] commented 3 years ago

Codecov Report

Merging #567 (4a78522) into master (0fe8731) will decrease coverage by 0.00%. The diff coverage is 93.75%.

@@            Coverage Diff             @@
##           master     #567      +/-   ##
==========================================
- Coverage   98.38%   98.38%   -0.01%     
==========================================
  Files          75       75              
  Lines        8411     8410       -1     
==========================================
- Hits         8275     8274       -1     
  Misses        136      136

Impacted Files	Coverage Δ
strawberryfields/backends/tfbackend/ops.py	`98.05% <92.85%> (-0.01%)`	:arrow_down:
strawberryfields/backends/tfbackend/backend.py	`100.00% <100.00%> (ø)`

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 0fe8731...4a78522. Read the comment docs.

co9olguy commented 3 years ago

Hi @Aaron-Robertson, just wanted to check the status of this PR. Any questions the team could help answer?

Aaron-Robertson commented 3 years ago

Hey @co9olguy, this one covers the low hanging cleanup items so far. I want to improve the partial trace but haven't gotten a chance to try much beyond research so far. The rest of the todo marked items in the tfbackend are tied to that partial trace, so if there's a recommendation for path forward I plan to take a shot at it this weekend.

Otherwise, I figure I could have more specific questions next week once I try some things.

co9olguy commented 3 years ago

No worries @Aaron-Robertson :slightly_smiling_face:

We're also happy to accept piecewise PRs (not all cleanup items need to be in one PR), so long as the updates are independent

Aaron-Robertson commented 3 years ago

Fair enough, I'll get this one ready to merge then and can start a separate one for the heavier lift 😄

Aaron-Robertson commented 3 years ago

@josh146 I think this one's ready to go. Merged master down with latest changes and it looks like the latest black version is throwing an error on comment format where it wasn't before (in gaussianbackend). Can fix here on request.