baggepinnen / FluxOptTools.jl

Use Optim to train Flux models and visualize loss landscapes

MIT License

59 stars 4 forks source link

Bump Flux dependency to latest (v0.12.3) #8

Closed AlexLewandowski closed 3 years ago

AlexLewandowski commented 3 years ago

This is enough to get the example in the README working and is a very minor fix. I don't know Zygote well enough to understand why the grads have Main.x and Main.y variables of type GlobalRef, but I have excluded them because they cause errors in veclength.

baggepinnen commented 3 years ago

Thanks for having a look at this! I tried running the tests locally, not sure why they didn't run here (tried to close/open this PR to trigger them..), but they failed rather Immediately.

I have not really maintained this package lately since there is supposed to be built-in functionality replacing the copyto in this package. Have you seen https://github.com/baggepinnen/FluxOptTools.jl/issues/4 ? I've never used the built-in functionality, but if it works, maybe we should update to use that instead of reimplementing it here.

AlexLewandowski commented 3 years ago

The tests related to FluxOptTools pass, at least on my end. Zygote can't handle the x variable from LinRange, it needs to be collected: x = collect(LinRange(-pi,pi,100)') which fixes the broadcasting error.

I did not know about Flux.destructure though, that does provide a stable API in case things change within Flux.

baggepinnen commented 3 years ago

I pushed an update to use GH actions, apparently this repo still had travis set up (which is no longer working).

codecov[bot] commented 3 years ago

Codecov Report

Merging #8 (1172eef) into master (16164bf) will decrease coverage by 0.58%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master       #8      +/-   ##
==========================================
- Coverage   97.16%   96.58%   -0.59%     
==========================================
  Files           2        2              
  Lines         106      117      +11     
==========================================
+ Hits          103      113      +10     
- Misses          3        4       +1

Flag	Coverage Δ
unittests	`96.58% <100.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/FluxOptTools.jl	`94.44% <100.00%> (-1.02%)`	:arrow_down:
src/SLBFGS.jl	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 50d64f2...1172eef. Read the comment docs.

baggepinnen commented 3 years ago

Thank you for your PR :)