tensorflow / tfjs

A WebGL accelerated JavaScript library for training and deploying ML models.
https://js.tensorflow.org
Apache License 2.0
18.47k stars 1.93k forks source link

Add dropout to tfjs-core. #117

Closed nsthorat closed 5 years ago

nsthorat commented 6 years ago

From @akshay951228 on February 6, 2018 5:27

I'm importing tensor model weight in DLJs, in my model I have dropout but dropout is not implemented yet.So my question is when it will be implemented

Copied from original issue: tensorflow/tfjs-core#659

nsthorat commented 6 years ago

From @dsmilkov on February 8, 2018 23:56

Dropout is a great example for an external contribution. Happy to provide review!

nsthorat commented 6 years ago

@caisq is this something that would be easy to push into deeplearnjs?

nsthorat commented 6 years ago

From @caisq on February 9, 2018 2:37

@nsthorat Feel free to assign this FR to me.

nsthorat commented 6 years ago

From @dsmilkov on February 9, 2018 2:57

Awesome. Trying to assign you but looks like we need to give you write access - will do now

nsthorat commented 6 years ago

From @abrad1212 on February 23, 2018 3:40

Couldn't you implement Dropout with everything in the current API?

Wouldn't it be similar to Tensorflow's implementation?

nsthorat commented 6 years ago

From @caisq on February 23, 2018 3:42

@abrad212 thanks for the ping. I have this on my TODO list

nsthorat commented 6 years ago

From @abrad1212 on February 23, 2018 3:44

@caisq No problem. Anything to help 😄

nsthorat commented 6 years ago

From @abrad1212 on February 23, 2018 17:40

@caisq I'm traveling today, and I think I have time over this weekend to try to implement DropOut. Hopefully, the implementation will be similar to the link I posted above, and translate easy. I'll let you know if I make any progress.

nsthorat commented 6 years ago

From @acostin1 on February 25, 2018 1:46

Would love to test it, I think I need to use dropout layers because my model is over-fitting too fast. Also, let's make sure one can use it in the model-builder demo :)

nsthorat commented 6 years ago

From @kwhitley on March 14, 2018 20:51

Agreed re. all... definitely need dropout implementation stat. The basic models without dropout, regularization, noise, etc overfit like a champ.

😞

nsthorat commented 6 years ago

Coming soon, I promise!

nsthorat commented 6 years ago

From @caisq on April 1, 2018 15:21

FYI, with the release of TensorFlow.js, now one can do dropout at the Layers level with tf.layers.dropout: https://js.tensorflow.org/api/0.6.1/#layers.dropout

As such, I'm removing the P1 tag from this bug. Dropout can later be pushed down to tfjs-core, so that those who don't use tfjs-layers can also use it.

VariableVasasMT commented 6 years ago

picking up this issue

caisq commented 6 years ago

@VariableVasasMT Thanks for volunteering. Please note that the code for dropout exists in the tf.layers.Dropout layer in the tfjs-layers repo. I think the code can be migrated to tfjs-core with relatively minor changes.

euler16 commented 6 years ago

@VariableVasasMT are you still working on this issue?

VariableVasasMT commented 6 years ago

@euler16 yes, I am still working on it, as suggested by @caisq, I was reading the code to understand better but I would be raising a pull request soon.

VariableVasasMT commented 6 years ago

@nsthorat dropout extends Layer, my question would be do I move Layer to core as well or change what it extends, and remove all the dependencies of tf.layers

nsthorat commented 6 years ago

You want to remove all the dependencies of tf.layers -- it shouldn't just be a code transplant, rather a reimplementation with core ops. Does that make sense?

VariableVasasMT commented 6 years ago

You want to remove all the dependencies of tf.layers -- it shouldn't just be a code transplant, rather a reimplementation with core ops. Does that make sense?

@nsthorat That's fair, I am worried about two codes which do the same thing and are very similar would exist parallelly. I am pro for reimplementation with core ops, just wanted to confirm once.

nsthorat commented 6 years ago

Once the core op is implemented we can call into it from layers :)

VariableVasasMT commented 6 years ago

@nsthorat please have a look into the pull request https://github.com/tensorflow/tfjs-core/pull/1343, I know it might not be correct, I was confused at a lot of places, but I think these are the right codes to move. I also moved getScalar, once they are moved to core, we can change the code in layers to adapt to these changes

VariableVasasMT commented 5 years ago

@nsthorat please review the updated the pull request.

syt123450 commented 5 years ago

Hi @nsthorat @caisq @dsmilkov , I sent a followup PR tfjs-core#1782 to make dropout op support non-default noiseShape, could you please take a look into it? Thanks!