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

Julia 0.7 #269

Merged
merged 32 commits into from
Aug 3, 2018
Merged

Julia 0.7 #269

merged 32 commits into from
Aug 3, 2018

Conversation

MikeInnes
Copy link
Member

@MikeInnes MikeInnes commented May 22, 2018

You can try this out with add Flux#julia-0.7.

CuArrays also needs updates, but Flux itself will work fine without that for now.

@ViralBShah
Copy link
Member

Can start with the update for 0.7. Tagging will be any day now.

@dfdx
Copy link

dfdx commented May 31, 2018

For broadcasting see JuliaAttic/TakingBroadcastSeriously.jl#8 - in Julia 0.7 overloading it became much easier.

@MikeInnes
Copy link
Member Author

It's a bit more complex than that for us, because we don't actually want to un-fuse broadcasts, and in fact 0.6 was simpler in this case. We have to inject dual numbers into the leaves of the Broadcasted tree (what previously would just have been a tuple of arguments), collect the partials and push them back through the original TrackedArray leaves.

@jekbradbury
Copy link
Contributor

I think you can use Broadcast.flatten to get something closer to the 0.6 interface with pre-fused broadcasts?

@MikeInnes MikeInnes force-pushed the julia-0.7 branch 5 times, most recently from da8d113 to 7b44adb Compare June 20, 2018 15:04
@MikeInnes
Copy link
Member Author

Yeah, doing that works for broadcast as long as we're happy to force fusion; though it's hard to imagine where that would be an issue.

@gustafsson would you be able to look at how we overload cat on 0.7? We're not catching things right now, looks like the methods we need to disambiguate has changed somewhat.

@MikeInnes MikeInnes force-pushed the julia-0.7 branch 2 times, most recently from 17b3313 to 14c870a Compare July 12, 2018 19:42
fs = fs == nothing ? [] : [:($(map(QuoteNode, fs.args)...),)]
:(treelike(@__MODULE__, $(esc(T)), $(fs...)))
end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your macro-fu is second to none, but isn't the @__MODULE__ stuff only needed because of your original strategy of making a function that uses @eval to behave kind of like a macro? Could you accomplish the same thing with just a macro and leave the treelike function only for the deprecation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good thought, and it'd work when the fields are explicitly provided, but otherwise we want to do fieldnames(T) where T is a type, rather than a symbol. So we have to go through at least one round of eval here.

@zenna
Copy link

zenna commented Jul 26, 2018

The MNIST example in ModelZoo is broken on .7 branch.
Flux.back! gives an out of memory error (which it didn't with 0.6), if I reduce the data (as in the following snippet) I get the error:

ERROR: DimensionMismatch("A has dimensions (100,32) but B has dimensions (10,100)")

Note that only occurs in the back!, m(X) works as expected

using Flux, Flux.Data.MNIST
using Flux: onehotbatch, argmax, crossentropy, throttle
using Base.Iterators: repeated
# using CuArrays

# Classify MNIST digits with a simple multi-layer-perceptron

imgs = MNIST.images()[1:100]
# Stack images into one large batch
X = hcat(float.(reshape.(imgs, :))...) |> gpu

labels = MNIST.labels()[1:100]
# One-hot-encode the labels
Y = onehotbatch(labels, 0:9) |> gpu

m = Chain(
  Dense(28^2, 32, relu),
  Dense(32, 10),
  softmax) |> gpu

loss(x, y) = crossentropy(m(x), y)
Flux.back!(loss(X, Y))

@SimonDanisch
Copy link
Contributor

Woot woot:

Test Summary: | Pass  Total
Flux          |  313    313
   Testing Flux tests passed 

I run into some annoying issue with Gzip, though.
The combination of JuliaLang/julia#22828 And the shaky "build" in:

https://github.com/JuliaIO/GZip.jl/blob/b8b8d897bba62b3b0bcc373064b0f4f84a32bd18/src/zlib_h.jl#L5

Makes gzip (and so Flux) pretty flaky.

I'm a bit confused by all the names (gzip, zlib, libz), but could https://github.com/bicycle1885/CodecZlib.jl be a replacement for Gzip? It uses that sweet BinaryProvider sauce ;)

@MikeInnes
Copy link
Member Author

I think I tried that at some point and didn't use it for some reason. Can't remember now though. If it's an issue I'm happy to switch over.

@MikeInnes MikeInnes merged commit 92d7e46 into master Aug 3, 2018
@JBlaschke
Copy link

Just a heads up, on macOS 10.13.6 and Julia v0.7 I get:

julia> import Flux
[ Info: Precompiling Flux [587475ba-b771-5e3f-ad9e-33799f191a9c]
ERROR: LoadError: LoadError: error compiling top-level scope: could not load library "libz"
dlopen(libz.dylib, 1): image not found

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

Successfully merging this pull request may close these issues.

9 participants