Skip to content

Conversation

@Fe-r-oz
Copy link
Contributor

@Fe-r-oz Fe-r-oz commented May 17, 2025

Hi, @apkille, I hope you are doing well. Sorry for taking a long time getting back to this PR. I followed the letter and spirit your suggestion: #51 (comment) and implemented the fix and tried it for random Gaussian objects. With this, there is no inference issue that we saw earlier and it works for all the required cases:

julia> randunitary(SArray, QuadPairBasis(1))
GaussianUnitary for 1 mode.
  symplectic basis: QuadPairBasis
displacement: 2-element SVector{2, Float64} with indices SOneTo(2):
 0.876015162384601
 0.3329954658579315
symplectic: 2×2 SMatrix{2, 2, Float64, 4} with indices SOneTo(2)×SOneTo(2):
 -1.42179  0.923228
 -1.52919  0.289627

julia> randunitary(SVector, SMatrix, QuadPairBasis(1))
GaussianUnitary for 1 mode.
  symplectic basis: QuadPairBasis
displacement: 2-element SVector{2, Float64} with indices SOneTo(2):
 0.43337635674542996
 0.5964111844829015
symplectic: 2×2 SMatrix{2, 2, Float64, 4} with indices SOneTo(2)×SOneTo(2):
 -1.01888   1.43857
 -0.832577  0.194057

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.7525975698565875
 0.812388421950281
symplectic: 2×2 SMatrix{2, 2, Float64, 4} with indices SOneTo(2)×SOneTo(2):
 -1.17787   1.17953
 -0.6919   -0.156116

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.32348195
 0.9691118
symplectic: 2×2 SMatrix{2, 2, Float32, 4} with indices SOneTo(2)×SOneTo(2):
  0.180392  0.969012
 -0.872356  0.85745

I have tested it as well and tried this approach for random gaussian objects, so I can push it and get your thoughts and suggestions on this! Please help review this PR, Thank you!

@codecov
Copy link

codecov bot commented May 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
ext/StaticArraysExt/StaticArraysExt.jl 100.00% <ø> (ø)
ext/StaticArraysExt/utils.jl 96.15% <100.00%> (+12.82%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@apkille apkille self-requested a review May 19, 2025 02:29
Copy link
Member

@apkille apkille left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Feroz, this LGTM and is much cleaner. Can you fix the method definitions so that they are written more in the standard Julian way? Also can you increase the codecov?


# % random.jl

randunitary(::Type{SArray}, basis::SymplecticBasis{N}; passive = false, ħ = 2) where {N<:Int} =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the need for let blocks here? Can't you just define a multi-line function in the usual way?

@Fe-r-oz Fe-r-oz requested a review from apkille June 6, 2025 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants