-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add @autostruct
macro
#22
Conversation
Nice! |
Niiiice! Comments:
|
My reservation is that the code inside is function-body code, not code you would normally find under julia> :(@mac struct Something
a, b = eachcol(rand(3,3))
for i in 1:3
a[i] = i^2
end
return a+b
end)
:(#= REPL[165]:1 =# @mac struct Something
#= REPL[165]:2 =#
(a, b) = eachcol(rand(3, 3))
#= REPL[165]:3 =#
for i = 1:3
#= REPL[165]:4 =#
a[i] = i ^ 2
#= REPL[165]:5 =#
end
#= REPL[165]:6 =#
return a + b
end)
julia> struct Something
a, b = eachcol(rand(3,3))
for i in 1:3
a[i] = i^2
end
return a+b
end
3-element Vector{Float64}:
1.3967202370899579
4.078791996072846
9.943963169620071
julia> methods(Something)
# 0 methods for type constructor Nevertheless this seems a bit strange to me. I like that if you delete the macro, and instead supply the struct definition yourself, then the function is a valid constructor. It's not a particularly magical macro. (Whether the name
It would certainly be easy to provide two macros -- say sharing the same inner function IDK if this is the right package for a non-Flux one though. Do you have non-Flux use cases in mind? Literally composing macros tends to work badly. But one can pretend by having in e.g.
If this ends up being useful, then I agree that might be a tidier final situation. I wonder if it ever matters besides being tidy -- will there be situations when performance differs, etc? Instead of a variant of the macro, the other way to make things permanent is to delete the macro, and add an explicit |
wow, weird stuff, that should be avoided. The I like the fact that
That's a good point. Wouldn't be hard to replace |
I was just looking forward to having a general-purpose package hosted by FluxML but also anywhere else. Also minimalistic versions like @autostruct A(x, y)
@autostruct B(x::Int, y = 2) would be useful. This makes me think that we could allow the syntax |
Could do. My examples all have only one constructor, and one reason to add constraints is to check other constructors. Doing the obvious thing does not work: julia> MyModel(d1::Int, d2::Int) = MyModel(Dense(d1 => d2), Dense(d2 => d1)) # does not work
ERROR: cannot define function MyModel; it already has a value
julia> MyModel
var"MyModel#13"
julia> var"MyModel#13"(d1::Int, d2::Int) = MyModel(Dense(d1 => d2), Dense(d2 => d1)) # does work!
var"MyModel#13"
julia> MyModel(2, 3)
MyModel(...) # 9 parameters, plus 8 non-trainable If you have multiple constructors all using the macro... same struct if they agree exactly on field names, but if not, you'll get different structs. I think?
I guess we can keep one eye on general-purpose usefulness. What I think this can't do is be type-stable through construction -- which ProtoStructs.jl manages:
|
I think making prototyping possible would be important for this. Similarly to proto structs, maybe just have the macro turn it into a named tuple wrapper? That way, every single struct would be
and you would simply update the tuple passed. Then you can re-run it again with different numbers of arguments, and it won’t matter |
There where some problems with Zygote + Functors with that approach |
I don't know if that was clear, but this PR's approach is fine for prototyping |
name could be |
speculative: we can also add a macro |
Re printing... there's now an option to do julia> @autostruct :expand function MyModel(d::Int)
...
julia> my = Chain(MyModel(2), Dense(2=>2))
Chain(
##MyModel#270(
Dense(2 => 2, tanh), # 6 parameters
Dense(2 => 2, tanh), # 6 parameters
),
Dense(2 => 2), # 6 parameters
) # Total: 6 arrays, 18 parameters, 384 bytes. IDK if that should be here, or if we should just make it so that you can call both macros (currently this doesn't work):
The macro can see these arguments already (it just doesn't look). It could store their values as a special field of the generated struct, to print However, I'm a little scared of edge cases this will invite. Someone will do |
Here's a failure case of the current code: julia> @autostruct :expand function New2(a, b)
A = Dense(a => b)
B = Dense(b => a)
return New2(A, B)
end
var"##New2#262"
julia> New2(1, 2)
ERROR: MethodError: no method matching Dense(::Pair{Dense{typeof(identity), Matrix{…}, Vector{…}}, Dense{typeof(identity), Matrix{…}, Vector{…}}})
Stacktrace:
[1] var"##New2#262"(a::Dense{typeof(identity), Matrix{…}, Vector{…}}, b::Dense{typeof(identity), Matrix{…}, Vector{…}})
@ Main ./REPL[4]:2
[2] var"##New2#262"(a::Int64, b::Int64)
@ Main ./REPL[4]:4
julia> methods(New2) # only the first seems to be called
# 2 methods for type constructor:
[1] var"##New2#262"(a, b)
@ REPL[4]:1
[2] var"##New2#262"(A::var"T#1", B::var"T#2") where {var"T#1", var"T#2"}
@ ~/.julia/dev/Fluxperimental/src/autostruct.jl:110
julia> @autostruct :expand function New3(a::Int, b::Int)
A = Dense(a => b)
B = Dense(b => a)
return New3(A, B)
end
var"##New3#263"
julia> New3(4, 5) # with a::Int, there is no ambiguity
New3(
Dense(4 => 5), # 25 parameters
Dense(5 => 4), # 24 parameters
) # Total: 4 arrays, 49 parameters, 404 bytes.
Or, as done in 12270a0, it can add enough extra fields which store julia> @autostruct :expand function New2(a, b)
A = Dense(a => b)
B = Dense(b => a)
return New2(A, B)
end
var"##New2#276"
julia> New2(1, 2)
New2(
Dense(1 => 2), # 4 parameters
Dense(2 => 1), # 3 parameters
nothing,
) # Total: 4 arrays, 7 parameters, 236 bytes.
julia> methods(New2)
# 2 methods for type constructor:
[1] var"##New2#276"(a, b)
@ REPL[54]:1
[2] var"##New2#276"(A::var"T#1", B::var"T#2", _nothing_1::Nothing) where {var"T#1", var"T#2"}
@ ~/.julia/dev/Fluxperimental/src/autostruct.jl:125 |
@@ -12,7 +12,7 @@ Zygote = "e88e6eb3-aa80-5325-afca-941959d7151f" | |||
|
|||
[compat] | |||
Compat = "4" | |||
Flux = "0.13.7, 0.14" | |||
Flux = "0.14.23" |
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.
Flux = "0.14.23" | |
Flux = "0.14" |
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.
As I guess you saw, it extends some printing methods which are only recently added.
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.
right, 0.14.23 was correct.
Since Flux has julia v1.10 lower bound, we should do the same here.
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.
Thanks for tagging, maybe it will work now...
I would be conservative and require to specify some argument's type in the constructor when the type has the same number of field. This is what you would do when not using / removing |
It's true that we could leave it up to you. But perhaps it's more mysterious when things fail and there is no visible Perhaps |
This is an alternative to
@compact
for easily defining layers. Instead of rolling everything into one,@autostruct function MyModel
takes the constructor function and magically defines the correspondingstruct
. You must still define the forward pass(m::MyModel)(x) = ...
as usual.The struct has an internal name. Iff you change the line
MyModel(dense1, dense2)
, it will define a new struct to match the new definition. The bindingMyModel = var"MyModel#13"
is notconst
, but this only affects construction: calling the model is still type-stable. (The struct always has type parameters.)