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

Update deps & bump to 0.16.1 #2574

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update deps & bump to 0.16.1 #2574

wants to merge 1 commit into from

Conversation

pxl-th
Copy link
Member

@pxl-th pxl-th commented Jan 4, 2025

PR Checklist

  • Tests are added
  • Entry in NEWS.md
  • Documentation, if applicable

Copy link

codecov bot commented Jan 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 32.50%. Comparing base (101106e) to head (71f53ab).
Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2574   +/-   ##
=======================================
  Coverage   32.50%   32.50%           
=======================================
  Files          34       34           
  Lines        2003     2003           
=======================================
  Hits          651      651           
  Misses       1352     1352           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mcabbott
Copy link
Member

mcabbott commented Jan 4, 2025

CI seems to get ⌃ [e88e6eb3] Zygote v0.6.75 not 0.7... maybe Optimisers.jl is to blame.

NNlib has errors FluxML/NNlib.jl#619

@pxl-th
Copy link
Member Author

pxl-th commented Jan 5, 2025

It's because of ComponentArrays used in tests. It needs a bump for [email protected].

UPD: Ah and optimisers as well.

@CarloLucibello
Copy link
Member

Ok, we are now picking Zygote 0.7.1 but there are some real failures

@pxl-th
Copy link
Member Author

pxl-th commented Jan 9, 2025

I think conv failures should be fixed by FluxML/NNlib.jl#620

@CarloLucibello
Copy link
Member

Now tests are not passing due to g containing thunks in the following example

julia> m = Chain([Dense(3 => 4, tanh; bias=false), Dense(4 => 2)]);

julia> x = randn(Float32,3,5);

julia> y = rand(Bool,2,5);

julia> g = gradient(m -> Flux.logitcrossentropy(m(x), y), m)[1]
(layers = NamedTuple{(:weight, :bias, )}[(weight = InplaceableThunk(ChainRules.var"#..., Thunk(ChainRules.var"#...)), bias = nothing, σ = nothing), (weight = Thunk(Zygote.var"#...), bias = Float32[-0.088241935, 0.088241935], σ = nothing)],)

@mcabbott
Copy link
Member

mcabbott commented Jan 9, 2025

Maybe Flux.gradient should recursively un-thunk?

@CarloLucibello
Copy link
Member

I wouldn't expect any thunk to appear in the output of Zygote.gradient as well, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants