-
Notifications
You must be signed in to change notification settings - Fork 150
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
Code suggestions for #1210 #1213
Code suggestions for #1210 #1213
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good idea, thanks for proposing that. Could you add some tests for the new functionality and safeguards for ex.args
indexing (like the one I've proposed)?
Should I update |
Yes, I think it would be good to update these two macros too. |
Hmm, I'm thinking of reusing macro SVector(ex)
ex = static_array_gen(SArray, ex, __module__)
:(SVector($ex))
end is almost fine, but some tests fail.. |
I have updated |
Oooh, I didn't know that julia> n=3; @SVector rand(n) # current master branch
3-element SVector{3, Float64} with indices SOneTo(3):
0.41421419554720196
0.8417028007187902
0.05502353350873945 I assumed all of the array sizes must be integers but not variables in this PR. julia> n=3; @SVector rand(n) # on this PR
ERROR: LoadError: @SVector got bad expression: rand(n)
Stacktrace:
[...] I noticed this behavior when testing Lines 283 to 284 in 67d9c36
|
Yes, that's why some macro variants generate |
I'll update this PR this weekend:muscle: |
Sorry for the late. I think this is ready to review after CI results turn green! |
Thanks! I will check this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks great, thanks!
80f5136
into
JuliaArrays:mbaran/fix-rand-generator
Thanks for the quick review! 😄 |
* 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 #1210 (#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]>
* 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]>
Overview
This PR adds suggestions for #1210.
@SArray rand(rng, 3)
@macroexpand @SArray rand(rng, Float32, 4)
@SArray rand(rng, Float32, Float32)
randn
andrandexp
Details
Add support for
@SArray rand(rng, 3)
On the original PR #1210
On this PR
Simple output for
@macroexpand @SArray rand(rng, Float32, 4)
On the original PR #1210
On this PR
Add support for
randn
andrandexp
On the original PR #1210
On this PR
Note
@SVector
andSMatrox
._rand_with_Val
is based on the comment (Extend macros with rand to support custom samplers #1210 (comment)).