Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend macros with rand to support custom samplers #1210

Merged
merged 10 commits into from
Jan 3, 2024

Conversation

mateuszbaran
Copy link
Collaborator

Fixes #804 .

@hyrodium
Copy link
Collaborator

hyrodium commented Oct 27, 2023

Thanks for the PR!

julia> using StaticArrays

julia> @SVector rand(1:5, 3)
3-element SVector{3, Int64} with indices SOneTo(3):
 1
 4
 4

julia> @macroexpand @SVector rand(1:5, 3)
:(StaticArrays._rand((StaticArrays.Random).GLOBAL_RNG, 1:5, StaticArrays.Size(3), (SVector){3}))

It works fine for the original issue, but I have some questions about the implementation.

These are why I proposed strand function in the comment. (#804 (comment))

Project.toml Outdated Show resolved Hide resolved
@mateuszbaran
Copy link
Collaborator Author

  • How can we create SizedArray version of rand(1:5, 3)?

I'd just do SizedVector{3}(rand(1:5, 3)).

These are why I proposed strand function in the comment. (#804 (comment))

I don't see this PR as a full replacement of strand, just a quick fix for the most commonly requested use case. If you want to work on strand, I can review your PR.

Co-authored-by: Yuto Horikawa <[email protected]>
test/arraymath.jl Outdated Show resolved Hide resolved
src/SVector.jl Outdated Show resolved Hide resolved
mateuszbaran and others added 2 commits October 27, 2023 20:47
Co-authored-by: Yuto Horikawa <[email protected]>
Co-authored-by: Yuto Horikawa <[email protected]>
Copy link
Collaborator

@hyrodium hyrodium left a comment

Choose a reason for hiding this comment

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

Could you please also update @SArray and @MArray macros? Other parts LGTM!

src/SMatrix.jl Outdated Show resolved Hide resolved
@mateuszbaran
Copy link
Collaborator Author

Could you please also update @SArray and @MArray macros? Other parts LGTM!

Sure. When working on it, I had an idea for supporting passing rng as well. What do you think?

@hyrodium
Copy link
Collaborator

Great!!:rocket: But I found some degression.

Before this PR

julia> using StaticArrays

julia> @SArray rand(3)
3-element SVector{3, Float64} with indices SOneTo(3):
 0.3872905952155763
 0.9519315858388775
 0.42729797815514925

After this PR

julia> using StaticArrays

julia> @SArray rand(3)
ERROR: LoadError: BoundsError: attempt to access 2-element Vector{Any} at index [3]
Stacktrace:

If we can handle rng in @SArray, it would be nice to have these features in @SVector and @SMatrix.

@mateuszbaran
Copy link
Collaborator Author

OK, I think I've fixed that issue and also added rng support to SVector and SMatrix.

src/SArray.jl Outdated Show resolved Hide resolved
hyrodium and others added 2 commits January 2, 2024 20:49
* move `ex.args[2] isa Integer`

* split `if` block

* simplify :zeros and :ones

* refactor :rand

* refactor :randn and :randexp

* update comments

* add _isnonnegvec

* update with `_isnonnegvec`

* add `_isnonnegvec(args, n)` method to check the size of `args`

* fix `@SArray` for `@SArray rand(rng,T,dim)` etc.

* update comments

* update `@SVector` macro

* update `@SMatrix`

* update `@SVector`

* update `@SArray`

* introduce `fargs` variable

* avoid `_isnonnegvec` in `static_matrix_gen`

* avoid `_isnonnegvec` in `static_vector_gen`

* remove unnecessary `_isnonnegvec`

* add `_rng()` function

* update tests on `@SVector` macro

* update tests on `@MVector` macro

* organize test/MMatrix.jl and test/SMatrix.jl

* organize test/MMatrix.jl and test/SMatrix.jl

* update with broken tests

* organize test/MMatrix.jl and test/SMatrix.jl for `rand*` functions

* fix around `broken` key for `@test` macro

* fix zero-length tests

* update `test/SArray.jl` to match `test/MArray.jl`

* update tests for `@SArray ones` etc.

* add supports for `@SArray ones(3-1,2)` etc.

* move block for `fill`

* update macro `@SArray rand(rng,2,3)` to use ordinary dispatches

* update around `@SArray randn` etc.

* remove unnecessary dollars

* simplify `@SArray fill`

* add `@testset "expand_error"`

* update tests for `@SArray rand(...)` etc.

* fix bug in `rand*_with_Val`

* cleanup tests

* update macro `@SMatrix rand(rng,2,3)` to use ordinary dispatches

* update macro `@SVector rand(rng,3)` to use ordinary dispatches

* move block for `fill`

* simplify `_randexp_with_Val`
@mateuszbaran mateuszbaran merged commit e2d772f into master Jan 3, 2024
22 of 29 checks passed
@mateuszbaran mateuszbaran deleted the mbaran/fix-rand-generator branch January 3, 2024 08:15
avik-pal pushed a commit to avik-pal/StaticArrays.jl that referenced this pull request Jan 12, 2024
* Extend macros with rand to support custom samplers

* Fix tests

* Update Project.toml

Co-authored-by: Yuto Horikawa <[email protected]>

* Update test/arraymath.jl

Co-authored-by: Yuto Horikawa <[email protected]>

* Update src/SVector.jl

Co-authored-by: Yuto Horikawa <[email protected]>

* Update src/SMatrix.jl

Co-authored-by: Yuto Horikawa <[email protected]>

* Update `SArray` macro

* fix reported issue; support rng in SArray and SMatrix

* Code suggestions for JuliaArrays#1210 (JuliaArrays#1213)

* move `ex.args[2] isa Integer`

* split `if` block

* simplify :zeros and :ones

* refactor :rand

* refactor :randn and :randexp

* update comments

* add _isnonnegvec

* update with `_isnonnegvec`

* add `_isnonnegvec(args, n)` method to check the size of `args`

* fix `@SArray` for `@SArray rand(rng,T,dim)` etc.

* update comments

* update `@SVector` macro

* update `@SMatrix`

* update `@SVector`

* update `@SArray`

* introduce `fargs` variable

* avoid `_isnonnegvec` in `static_matrix_gen`

* avoid `_isnonnegvec` in `static_vector_gen`

* remove unnecessary `_isnonnegvec`

* add `_rng()` function

* update tests on `@SVector` macro

* update tests on `@MVector` macro

* organize test/MMatrix.jl and test/SMatrix.jl

* organize test/MMatrix.jl and test/SMatrix.jl

* update with broken tests

* organize test/MMatrix.jl and test/SMatrix.jl for `rand*` functions

* fix around `broken` key for `@test` macro

* fix zero-length tests

* update `test/SArray.jl` to match `test/MArray.jl`

* update tests for `@SArray ones` etc.

* add supports for `@SArray ones(3-1,2)` etc.

* move block for `fill`

* update macro `@SArray rand(rng,2,3)` to use ordinary dispatches

* update around `@SArray randn` etc.

* remove unnecessary dollars

* simplify `@SArray fill`

* add `@testset "expand_error"`

* update tests for `@SArray rand(...)` etc.

* fix bug in `rand*_with_Val`

* cleanup tests

* update macro `@SMatrix rand(rng,2,3)` to use ordinary dispatches

* update macro `@SVector rand(rng,3)` to use ordinary dispatches

* move block for `fill`

* simplify `_randexp_with_Val`

---------

Co-authored-by: Yuto Horikawa <[email protected]>
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.

Generating random StaticArrays works with SamplerTypes but not other Samplers
3 participants