axon
axon copied to clipboard
Warn on non-MFA functions
Resolves #323
For now, just warns. Will raise on next release
@seanmor5 there is a limitation in this approach in that you cannot access closures. For example, imagine someone does a layer like this:
y = 1
layer(x, fn x -> x + y end)
If we force external functions, there is no way to access y
anymore. Maybe that's intentional, maybe you require all bindings to be given as layer arguments like this:
y = 1
layer({x, y}, fn {x, y} -> x + y end)
But in case it isn't, you probably want to revisit. :)
@josevalim I think that's okay because you still wouldn't be able to deserialize the closure correctly in another program?
In cases where you want more flexibility, you can just serialize the parameters and ensure you use the same code to generate the model programmatically
Yes, the closure is wrong either way. My concern is that we cannot reproduce the closure capabilities now, with the chosen API, unless we support an additional optional argument, called extra_args
, which we append to the existing args. So, for example, we could do:
layer(x, &SomeFun.bar/4, [y])
There is a workaround for this which I just pushed, but as you can see it is failing because there are some inconsistencies in generating parameter shapes. Basically if you want a closure you can just use Axon.constant layers as input :)
The issue now is that parameter shapes are functions which depend on the known input shapes at initialization time, which creates kind of weird scenarios where if you swap out a parameter for a constant than it breaks things. I think I need to update that to work regardless of whether a layer input is an Axon input or Parameter input
Perfect then, ship it!