XanaduAI / thewalrus

A library for the calculation of hafnians, Hermite polynomials and Gaussian boson sampling.
https://the-walrus.readthedocs.io
Apache License 2.0
99 stars 54 forks source link

Extend single mode symplectic to act on multiple modes 📏 #347

Closed sduquemesa closed 1 year ago

sduquemesa commented 1 year ago

Context: No method is provided to extend a single-mode symplectic to act on several modes.

Description of the Change:

Benefits:

Possible Drawbacks: None

Related GitHub Issues: None

codecov[bot] commented 1 year ago

Codecov Report

Merging #347 (59d8725) into master (d8def78) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #347   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        24           
  Lines         1725      1737   +12     
=========================================
+ Hits          1725      1737   +12     
Impacted Files Coverage Δ
thewalrus/symplectic.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 d8def78...59d8725. Read the comment docs.

nquesada commented 1 year ago

so this acts the same symplectic in all the modes?

sduquemesa commented 1 year ago

Thanks @ilan-tz and @nquesada,

so this acts the same symplectic in all the modes?

@nquesada Correct!


As per @ilan-tz comments:

In the current implementation, I believe extend operates in the xpxp convention rather than the default xxpp convention.

Resolved ✅

When I input a single mode as an argument to extend, I get a ValueError -- however, perhaps it should reproduce expand in this case, allowing me to act just on the mode I specify?

I've merged the two functions so you should see the same expected behaviour.

Currently, the entries corresponding to the modes on which the symplectic doesn't act are assumed to be 0 -- however, they should be the identity.

My bad, sorry 🙈 Resolved. 

Perhaps we should add this into expand after all, rather than have a separate function? What do you think?

Agree!