Updated model script formats #210

AdarshKumar712 · 2020-03-04T16:04:58Z

I have updated the script formats of the following models:

vision/mnist/mlp.jl
other/iris/iris.jl

I have also added Project.toml and Manifest.toml files for iris dataset. Will add Project.toml and Manifest.toml files for the mnist models as well, once there script format are updated. I am currently working on them. I will add them as well pretty soon. Please review these updates, and suggest further changes.

Update:
I have updated following models as well:

other/housing/housing.jl
vision/mnist/conv.jl
vision/mnist/vae.jl
vision/mnist/autoencoders.jl
vision/mnist/cifar10/cifar10.jl
vision/mnist/cppn/cppn.jl
vision/dcgan_mnist/dcgan.jl
other/fizzbuzz/fizzbuzz.jl
other/bitstring-parity/xor1.jl, xor2.jl, xor3.jl
text/char-rnn/char-rnn.jl
text/lang-detection/model.jl
text/phonemes/0-data.jl, 1-model.jl
text/treebank/data.jl, recursive.jl

DhairyaLGandhi · 2020-03-04T17:19:08Z

We should avoid adding environments per model, it's one thing that made the zoo very difficult to maintain

AdarshKumar712 · 2020-03-05T10:59:38Z

Ok, then I will remove local environment files from here, and from some other places as well, and instead will Update the global environment file.

CarloLucibello · 2020-03-05T16:05:50Z

We should avoid adding environments per model, it's one thing that made the zoo very difficult to maintain

Do you have a reference for this discussion? I would say quite the opposite: a general manifest for all of the examples in unmaintainable. No one will ever update all the scripts at the same time, so some of them will rot and some will go out of sync with the general manifest. Kind of the current situation

AdarshKumar712 · 2020-03-05T18:13:20Z

I have a doubt. In the Conv.jl, we are saving the parameters as:
BSON.@save joinpath(dirname(@__DIR__), "mnist_conv.bson") params=cpu.(params(model)) epoch_idx acc
How one is going reload the model using this way? Shouldn't we add how models can be reloaded as well?
@dhairyagandhi96 @CarloLucibello

other/iris/iris.jl

CarloLucibello · 2020-03-05T18:36:53Z

I have a doubt. In the Conv.jl, we are saving the parameters as:
BSON.@save joinpath(dirname(@__DIR__), "mnist_conv.bson") params=cpu.(params(model)) epoch_idx acc
How one is going reload the model using this way? Shouldn't we add how models can be reloaded as well?
@dhairyagandhi96 @CarloLucibello

you can use load_params!. It won't work correctly for models with buffers such as batch norm layers, since won't be saved and reloaded, but for a simple conv net it's fine

AdarshKumar712 · 2020-03-06T19:42:08Z

@dhairyagandhi96 @CarloLucibello I have updated few other models. Please have a look, and suggest changes wherever required.

DhairyaLGandhi · 2020-03-09T19:07:08Z

ref #173

DhairyaLGandhi · 2020-03-09T19:08:15Z

The general manifest is expected to work with patch updates, else its a bug, and a major update would largely require a rewrite anyway

CarloLucibello · 2020-03-13T12:22:57Z

The general manifest is expected to work with patch updates, else its a bug, and a major update would largely require a rewrite anyway

problem is that after a major update no one is going to undertake the update of the whole repo (we have just seen this). What we can hope for (and what we consistently got in the past months) are pointwise PRs updating single scripts. This is why we should just have one manifest per example to have both manageability and consistency

DhairyaLGandhi · 2020-03-13T12:26:08Z

We had that earlier and it lead to a case where we missed the guarantee that the models would update accordingly. Maintaining multiple environments was also error prone. This time the reason was that we waited for some Zygote bugs to be resolved, a large change which took time to stabilize

AdarshKumar712 · 2020-03-13T14:17:57Z

Sorry I had been busy writing my GSoC Proposal, because of which I wasn't able to work on this PR. Its almost done now, I will soon update format of other files as well.
Btw, @dhairyagandhi96 how about having individual Manifest and Project files for each section? Like Vision, Text, Others. Manifest.toml, Project.toml files for each one. This way it would be easier to maintain each section separately.

AdarshKumar712 · 2020-03-16T12:45:00Z

@CarloLucibello @dhairyagandhi96 I have updated all models in text, other and vision section, except for one flux-next as I was not clear about its purpose. Please do have a look at the changes and suggest improvements.

jamblejoe · 2020-03-26T02:03:03Z

Is the device keyword of the Args struct in mlp.jl ever used? Is the model actually trained on the gpu?

AdarshKumar712 · 2020-03-26T08:35:46Z

Sorry @jamblejoe, I haven't checked for the GPU support for all the scripts. This device keyword was left unused by mistake. I will update this shortly. Thanks for reminding! :)

DhairyaLGandhi · 2020-04-24T04:32:57Z

What remains to be done here?

AdarshKumar712 · 2020-04-24T06:08:56Z

@dhairyagandhi96 I have updated all the models except others/Flux-next and contrib section. All models work with CPU. But the text models throws an error with the GPU because of Onehotmatrix. Other than that vision models work fine with GPU.

Also for char-rnn.jl, the model performs better without reset command there. So I haven't included that in char-rnn.jl .

vision/mnist/mlp.jl

CarloLucibello · 2020-04-24T12:51:51Z

text/char-rnn/char-rnn.jl


    function loss(xs, ys)
-      l = sum(crossentropy.(m.(gpu.(xs)), gpu.(ys)))
+      l = sum(logitcrossentropy.(m.(xs), ys))


is it ok to have a broadcast here?

Yes, here without the broadcast, it throws the following error:

MethodError: no method matching isless(::Array{Float32,2}, ::Array{Float32,2}) Closest candidates are: isless(!Matched::Missing, ::Any) at missing.jl:87 isless(::Any, !Matched::Missing) at missing.jl:88

as here xs is an array of 50 elements where each element is a 2D array (N x 50) which is input for the model. However, I think here it would be better to have mean rather than sum. I was not much sure on that. So I left it as it was earlier.

CarloLucibello · 2020-04-24T12:52:54Z

text/phonemes/1-model.jl

@@ -79,7 +79,7 @@ function train(; kws...)
    @info("Constructing Model...")
    state, encode = Construct_model(args)

-    loss(x, yo, y) = sum(crossentropy.(model(x, yo, state, encode), y))
+    loss(x, yo, y) = sum(logitcrossentropy.(model(x, yo, state, encode), y))


also here, do we want to broadcast?

This also had the same problem as above.

CarloLucibello · 2020-04-30T07:03:25Z

ok, let's rebase and merge this

text/char-rnn/char-rnn.jl

vision/mnist/mlp.jl

vision/mnist/conv.jl

CarloLucibello · 2020-04-30T10:50:31Z

thanks for all this work!

MikeInnes · 2020-05-12T14:01:12Z

See #226 (comment), but also this PR did not update the project or manifest files, so the code does not currently run.

AdarshKumar712 · 2020-05-13T06:40:23Z

The project file was not updated earlier because it was unclear how were we going to handle project dependencies. Though Dhairya earlier pointed out(comment) that having many project files makes it harder to handle, however at the same time having a single project file makes it tough at the time of making updates(comment). To this I suggested that it would be better if we have a section-wise Project and Manifest file like for Vision, Text, Other. However, this discussion didn't go to a conclusion at that time, and remain undecided.
I apologize that I missed pointing it out earlier. I hope it won't take much time to update the project files once it's decided on how we are going to keep the project files.

MikeInnes · 2020-05-13T10:22:47Z

Having code in the zoo that needs a project file but doesn't have one definitely seems like the worst of all worlds.

AdarshKumar712 · 2020-05-13T16:51:22Z

I have added the Project files, #232

CarloLucibello reviewed Mar 5, 2020

View reviewed changes

other/iris/iris.jl Outdated Show resolved Hide resolved

AdarshKumar712 requested a review from CarloLucibello March 6, 2020 19:41

CarloLucibello mentioned this pull request Mar 14, 2020

mnist mlp.jl: ERROR: scalar getindex is disallowed #212

Closed

AdarshKumar712 force-pushed the Update-models branch from 6ed5fed to 7d3686e Compare March 15, 2020 14:46

AdarshKumar712 and others added 13 commits March 15, 2020 21:12

Updated mlp

5ee4925

Updated iris.jl

d4ce0cc

Minute comment corrections

fe7fa8e

Corrected indentations and comments

d59753c

Delete Manifest.toml

09eb885

Delete Project.toml

1cfc254

Updated housing.jl

11f6eb1

Updated conv.jl, vae.jl, iris.jl(logitcross)

9611541

Updated autoencoders.jl

9ab8776

Updated cifar10.jl format

5b69150

Updated cppn.jl, dcgan.jl, fizzbuzz.jl

f4edb1b

Updated bitstring-parity, char-rnn.jl, lang-detection/model.jl

72f7e34

Cleanup and rebasing

7d3686e

Updated phonemes model and treebank model

560a14a

Updated few models to support GPU, cleanup

b5eb653

AdarshKumar712 mentioned this pull request Mar 27, 2020

Cifar10 example with VGG: accuracy stuck at 0.087 #220

Open

Resolved Conflicts and some minor changes

17ca886

CarloLucibello reviewed Apr 24, 2020

View reviewed changes

vision/mnist/mlp.jl Outdated Show resolved Hide resolved

CarloLucibello reviewed Apr 24, 2020

View reviewed changes

Replaced crossentropy with logitcrossentropy

110f1ba

AdarshKumar712 force-pushed the Update-models branch from b3e1b45 to 110f1ba Compare April 24, 2020 14:17

AdarshKumar712 requested a review from CarloLucibello April 26, 2020 10:54

Rebase and cleanup

4c9018b

AdarshKumar712 force-pushed the Update-models branch from 030f4b7 to 4c9018b Compare April 30, 2020 08:55

CarloLucibello reviewed Apr 30, 2020

View reviewed changes

text/char-rnn/char-rnn.jl Outdated Show resolved Hide resolved

CarloLucibello reviewed Apr 30, 2020

View reviewed changes

text/char-rnn/char-rnn.jl Outdated Show resolved Hide resolved

CarloLucibello reviewed Apr 30, 2020

View reviewed changes

vision/mnist/mlp.jl Outdated Show resolved Hide resolved

Updated function names and added 'flatten' layer

ab152da

CarloLucibello reviewed Apr 30, 2020

View reviewed changes

vision/mnist/conv.jl Outdated Show resolved Hide resolved

update flatten

d0a3b13

CarloLucibello merged commit 4bf8476 into FluxML:master Apr 30, 2020

mchristianl mentioned this pull request Jan 16, 2022

re-establishing GPU support for char-rnn.jl #331

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated model script formats #210

Updated model script formats #210

AdarshKumar712 commented Mar 4, 2020 •

edited

Loading

DhairyaLGandhi commented Mar 4, 2020

AdarshKumar712 commented Mar 5, 2020

CarloLucibello commented Mar 5, 2020 •

edited

Loading

AdarshKumar712 commented Mar 5, 2020 •

edited

Loading

CarloLucibello commented Mar 5, 2020

AdarshKumar712 commented Mar 6, 2020 •

edited

Loading

DhairyaLGandhi commented Mar 9, 2020

DhairyaLGandhi commented Mar 9, 2020

CarloLucibello commented Mar 13, 2020

DhairyaLGandhi commented Mar 13, 2020

AdarshKumar712 commented Mar 13, 2020 •

edited

Loading

AdarshKumar712 commented Mar 16, 2020

jamblejoe commented Mar 26, 2020

AdarshKumar712 commented Mar 26, 2020

DhairyaLGandhi commented Apr 24, 2020

AdarshKumar712 commented Apr 24, 2020 •

edited

Loading

CarloLucibello Apr 24, 2020

AdarshKumar712 Apr 24, 2020 •

edited

Loading

CarloLucibello Apr 24, 2020

AdarshKumar712 Apr 24, 2020 •

edited

Loading

CarloLucibello commented Apr 30, 2020

CarloLucibello commented Apr 30, 2020

MikeInnes commented May 12, 2020 •

edited

Loading

AdarshKumar712 commented May 13, 2020 •

edited

Loading

MikeInnes commented May 13, 2020

AdarshKumar712 commented May 13, 2020

Updated model script formats #210

Updated model script formats #210

Conversation

AdarshKumar712 commented Mar 4, 2020 • edited Loading

DhairyaLGandhi commented Mar 4, 2020

AdarshKumar712 commented Mar 5, 2020

CarloLucibello commented Mar 5, 2020 • edited Loading

AdarshKumar712 commented Mar 5, 2020 • edited Loading

CarloLucibello commented Mar 5, 2020

AdarshKumar712 commented Mar 6, 2020 • edited Loading

DhairyaLGandhi commented Mar 9, 2020

DhairyaLGandhi commented Mar 9, 2020

CarloLucibello commented Mar 13, 2020

DhairyaLGandhi commented Mar 13, 2020

AdarshKumar712 commented Mar 13, 2020 • edited Loading

AdarshKumar712 commented Mar 16, 2020

jamblejoe commented Mar 26, 2020

AdarshKumar712 commented Mar 26, 2020

DhairyaLGandhi commented Apr 24, 2020

AdarshKumar712 commented Apr 24, 2020 • edited Loading

CarloLucibello Apr 24, 2020

Choose a reason for hiding this comment

AdarshKumar712 Apr 24, 2020 • edited Loading

Choose a reason for hiding this comment

CarloLucibello Apr 24, 2020

Choose a reason for hiding this comment

AdarshKumar712 Apr 24, 2020 • edited Loading

Choose a reason for hiding this comment

CarloLucibello commented Apr 30, 2020

CarloLucibello commented Apr 30, 2020

MikeInnes commented May 12, 2020 • edited Loading

AdarshKumar712 commented May 13, 2020 • edited Loading

MikeInnes commented May 13, 2020

AdarshKumar712 commented May 13, 2020

AdarshKumar712 commented Mar 4, 2020 •

edited

Loading

CarloLucibello commented Mar 5, 2020 •

edited

Loading

AdarshKumar712 commented Mar 5, 2020 •

edited

Loading

AdarshKumar712 commented Mar 6, 2020 •

edited

Loading

AdarshKumar712 commented Mar 13, 2020 •

edited

Loading

AdarshKumar712 commented Apr 24, 2020 •

edited

Loading

AdarshKumar712 Apr 24, 2020 •

edited

Loading

AdarshKumar712 Apr 24, 2020 •

edited

Loading

MikeInnes commented May 12, 2020 •

edited

Loading

AdarshKumar712 commented May 13, 2020 •

edited

Loading