-
-
Notifications
You must be signed in to change notification settings - Fork 604
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
Some small printing upgrades #2344
base: master
Are you sure you want to change the base?
Conversation
Is having the |
The idea is that printing the values of a huge matrix isn't helpful, but showing the size is. This is what the present printing of e.g. Dense achieves. But (It's not exactly summary as this gets quite long for e.g. |
Revisiting this, it would be nice to retain some more info such as eltype if the contents aren't printed. Though it's far from perfect, could we avoid a lot of complexity by setting the |
Latest commit keeps eltype as suggested (or really, the first type parameter): julia> struct Tmp2; x; y; end; Flux.@functor Tmp2
julia> Chain(Tmp2([Dense(2,3), randn(3,4)'], (x=1:3, y=Dense(3,4), z=rand(3))))
Chain(
Tmp2(
[
Dense(2 => 3), # 9 parameters
4×3 Adjoint{Float64,...}, # 12 parameters
],
(;
x = 3-element UnitRange{Int64}, # 3 parameters
y = Dense(3 => 4), # 16 parameters
z = 3-element Vector{Float64}, # 3 parameters
),
),
) # Total: 7 arrays, 43 parameters, 780 bytes. Note also that no model using existing layers is affected by this. The goal is mainly to make |
We were discussing this during our call this week. The consensus was that using There are times when the type (e.g. |
This is intended for FluxML/Fluxperimental.jl#20 but probably a good idea anyway. Motivating example is something like this (where bigger arrays will print pages of numbers):
Notice that
Array()
andNamedTuple()
aren't actually valid syntax. Also that it prints whole arrays if they aren't inside a layer the way it expects. After this PR: