fix: cleaner dispatching onto StaticArrays

[Fe-r-oz]

This PR aims to resolve #39 to provide cleaner dispatching onto StaticArrays. I hope that I have correctly followed this wonderful comment: #39 (comment)!

Edit: Dear Andrew, Thank you for your helpful and wonderful feedback 🙂 I really appreciate your insights. I wasn’t entirely sure whether you would prefer trait-based dispatch over type-based dispatch, but I found that traits provided a simpler and flexible solution. I would to be happy to hear your thoughts and criticism :) As mentioned here, type-based dispatch was causing complications such as method overwriting, code repetition, and unexpected errors for me! 😅 Dispatch on Traits, on the other hand, allowed for a neater approach by defining properties that types can have and dispatching accordingly. Additionally, we can easily extend this approach to PDMats as mentioned here. 😃

Outputs: 😃

julia> using Gabs;
julia> using StaticArrays;
julia> result1 = attenuator(QuadPairBasis(1), pi/6, 3);
julia> result1 = attenuator(Array, QuadPairBasis(1), pi/6, 3);
julia> result1 = attenuator(SArray, QuadPairBasis(1), pi/6, 3);
julia> result1 = attenuator(SVector, SMatrix, QuadPairBasis(1), pi/6, 3);
julia> result1 = attenuator(Vector, Matrix, QuadPairBasis(1), pi/6, 3);
julia> displace(SArray, QuadPairBasis(1), 0.1);

It was a wonderful learning experience for me. It helped me realize that dispatch on traits avoid code duplication as shown here and cleanly handle orthogonal properties (being a StaticArray, Array, or PDMat etc.), while dispatch on types requires adding new methods for each types that we add in future as well. I will ponder on why dispatch on types causes mistakes on my end 😅 Silly me...

I truly appreciate your guidance. I look forward to hearing your thoughts and criticism on which approach you feel would be best for the future! I may have overlooked some aspects or not fully considered the long-term implications of each approach 😅. That said, I truly enjoyed working on this! My apologies if I made any silly mistakes! Thank you! 🙂

[codecov]

Codecov Report

Attention: Patch coverage is 95.54455% with 9 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/utils.jl	50.00%	6 Missing ⚠️
ext/StaticArraysExt/utils.jl	81.25%	3 Missing ⚠️

Files with missing lines	Coverage Δ
ext/StaticArraysExt/StaticArraysExt.jl	100.00% <ø> (ø)
src/channels.jl	99.64% <100.00%> (+0.06%)	⬆️
src/randoms.jl	100.00% <100.00%> (ø)
src/states.jl	99.35% <100.00%> (+0.51%)	⬆️
src/unitaries.jl	99.77% <100.00%> (+0.01%)	⬆️
ext/StaticArraysExt/utils.jl	89.28% <81.25%> (-10.72%)	⬇️
src/utils.jl	66.66% <50.00%> (-16.67%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Fe-r-oz]

Please help review this PR, thank you so much!

Edit: Hi, Andrew! There were some merge conflict after latest commit, so I resolved them which led to some commits 🙏🏼

[Fe-r-oz]

Hi, Andrew! My apologies 🙏🏼 There were some after errors after I resolved the merge conflicts so I had to push some additional commits 😅

[apkille]

Hi Feroz! Thanks for this contribution! StaticArrays is such a handy resource for this package, so this is great. I left a few comments for this round of review. More generally, they are the following:

• A cleaner and more encompassing utility function for inferring the input array type(s). This can probability be something like

_infer_types(T1, T2, basis)
_infer_types(T, basis)

• Allowing a user to also just specify a single array type (e.g. SArray) if they're lazy and don't want to write SVector and SMatrix.

[Fe-r-oz]

Hi, Andrew!

Thank you so much for your wonderful and helpful suggestions! They really helped improved this PR!

Please help review this PR, thank you!

[apkille]

Hi Feroz, thanks so much again! A few things:

• I put in one of the comments a potential approach for dealing with the static arrays extension. Let me know if you think that's the way to go.
• We shouldn't have the previous tests for dispatching onto static arrays break. So, e.g., these implementations should all be able to work once this PR is merged:

displace(SVector{2n}, SMatrix{2n,2n}, basis, alpha)
displace(SVector, SMatrix, basis, alpha)
displace(SArray, basis, alpha)

• Could you also make sure these work for random Gaussian objects in src/randoms.jl? I could be wrong, but I don't think I saw changes for those methods in this PR.

[Fe-r-oz]

Hi, Andrew!

Thank you for your suggestions. Kindly please let me know your comments!

I have made sure that there are no breaking changes. All three points suggested in #51 (review) have been incorporated. There were some tests that are broken due to the same old symbolic error previously seen in #46 (comment).

Maybe plausible change: After taking the tensor product with with static arrays, the .disp, .transform and .noise are not static array which is corrected. Hopefully, this sounds reasonable.

It also appears that if the user is lazy and relies on SVector, SMatrix, etc., type inference follows suit, resulting in Any in the current context. While the current approach doesn't break anything and introduce new changes, it does seem to impact type inference due to user laziness, which appears to be a caveat.

julia> state = coherentstate(SVector, SMatrix, QuadPairBasis(1), 1.0-im)
GaussianState for 1 mode.
  symplectic basis: QuadPairBasis
mean: 2-element SVector{2, Any} with indices SOneTo(2):
  2.0
 -2.0
covariance: 2×2 SMatrix{2, 2, Any, 4} with indices SOneTo(2)×SOneTo(2):
 1.0  0.0
 0.0  1.0

julia> state = coherentstate(SVector{2, Float32}, SMatrix{2,2, Float32}, QuadPairBasis(1), 1.0-im)
GaussianState for 1 mode.
  symplectic basis: QuadPairBasis
mean: 2-element SVector{2, Float32} with indices SOneTo(2):
  2.0
 -2.0
covariance: 2×2 SMatrix{2, 2, Float32, 4} with indices SOneTo(2)×SOneTo(2):
 1.0  0.0
 0.0  1.0

[apkille]

Hi @Fe-r-oz, sorry for the late response. I've been thinking about this implementation and a cleaner way of doing it. What we could do, rather than inserting the _infer_types utility function into every predefined method that creates a Gaussian object (which might be confusing to new contributors), is dispatch onto the static arrays type in ext/StaticArraysExt for every predefined method. So, for instance, somewhere in that folder we could define for randunitary the methods

randunitary(::Type{SArray}, basis::SymplecticBasis{N}) where {N<:Int} = (n = basis.nmodes; randunitary(SVector{2n,Float64}, SMatrix{2n,2n,Float64}, basis))
randunitary(::Type{SVector}, ::Type{SMatrix}, basis::SymplecticBasis{N}) where {N<:Int} = (n = basis.nmodes; randunitary(SVector{2n,Float64}, SMatrix{2n,2n,Float64}, basis))
randunitary(::Type{SVector{M}}, ::Type{SMatrix{M,M}}, basis::SymplecticBasis{N}) where {M,N<:Int} = (n = basis.nmodes; randunitary(SVector{2n,Float64}, SMatrix{2n,2n,Float64}, basis))

which simply defines a more narrow dispatch for SArray and also avoids the problem of type inference:

julia> randunitary(SArray, QuadPairBasis(1))
GaussianUnitary for 1 mode.
  symplectic basis: QuadPairBasis
displacement: 2-element SVector{2, Float64} with indices SOneTo(2):
 0.7672717498076692
 0.12292186044654341
symplectic: 2×2 SMatrix{2, 2, Float64, 4} with indices SOneTo(2)×SOneTo(2):
  0.155984  1.56888
 -0.473889  1.64457

julia> randunitary(SVector, SMatrix, QuadPairBasis(1))
GaussianUnitary for 1 mode.
  symplectic basis: QuadPairBasis
displacement: 2-element SVector{2, Float64} with indices SOneTo(2):
 0.7048076969727619
 0.005874773130562838
symplectic: 2×2 SMatrix{2, 2, Float64, 4} with indices SOneTo(2)×SOneTo(2):
 -1.34124   0.689583
 -1.15812  -0.150146

julia> randunitary(SVector{2}, SMatrix{2,2}, QuadPairBasis(1))
GaussianUnitary for 1 mode.
  symplectic basis: QuadPairBasis
displacement: 2-element SVector{2, Float64} with indices SOneTo(2):
 0.34004513579276363
 0.6134814893574261
symplectic: 2×2 SMatrix{2, 2, Float64, 4} with indices SOneTo(2)×SOneTo(2):
  0.724417  0.508209
 -0.63204   0.937018

julia> randunitary(SVector{2,Float32}, SMatrix{2,2,Float32}, QuadPairBasis(1))
GaussianUnitary for 1 mode.
  symplectic basis: QuadPairBasis
displacement: 2-element SVector{2, Float32} with indices SOneTo(2):
 0.47838023
 0.28229055
symplectic: 2×2 SMatrix{2, 2, Float32, 4} with indices SOneTo(2)×SOneTo(2):
  0.414853  0.861266
 -0.744631  0.864581

To me, this seems like a cleaner and more Julian approach, as it's just carefully implementing multiple dispatch and not adding additional utility functions. Let me know your thoughts!

[Fe-r-oz]

Hi, Andrew!

Thank you for your insights. The latest approach that you suggested looks much cleaner and simpler than previous over-cooked approaches. I am excited to follow this approach!