Skip to content

Conversation

@BoltzmannZhaung
Copy link

Dear Post Dr.Bourgin,
I wanna give a perfect method code, however , unfortunately may be it can not work well (I have not test the method).So.....uh~~ may I ask your help to finish it, then get the honor that be a contributor of this great project (5k+ stars)....
THANK YOU!

@WuZhuoran
Copy link
Contributor

WuZhuoran commented Jul 13, 2019

Maybe you can refer keras's Alpha Dropout here to create unit test.

And PyTorch Version here.

@BoltzmannZhaung
Copy link
Author

How fantastic!!! Thank you~

@ddbourgin
Copy link
Owner

Hi @BoltzmannZhaung - thanks for doing this, on first glance it looks great :) Will take a closer look soon.

@BoltzmannZhaung
Copy link
Author

thank u~!

a = ((1 - self.p) * (1 + self.p * alpha_p ** 2)) ** -0.5
b = -a * alpha_p * self.p

keep = self._greaterOrEqual(np.random.uniform(0,1,X.shape))
Copy link
Owner

Choose a reason for hiding this comment

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

Does this run? It looks like you don't pass a y value to your _greaterOrEqual method.

In general, I don't think you need to create a separate method. A one-liner is still pretty clean (IMO): np.random.rand(*X.shape) >= self.p.

Copy link
Owner

Choose a reason for hiding this comment

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

More generally, this function is very similar to the Keras implementation here.

I want to stress that all code in this repo should be your own work, or should properly cite the code it's based on.

class AlphaDropout(WrapperBase):
def __init__(self, wrapped_layer, p):
"""
Alpha Dropout is a `Dropout` that aim at keeping mean and variance to
Copy link
Owner

Choose a reason for hiding this comment

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

It looks like this description is copied from the Tensorflow documentation. I think it's better if we have documentation be more explicit + verbose for a project like this, since the expectation is that these will be "reference" implementations for people to use when trying to understand how the different model components work.

With that in mind, this should be rewritten. I'm happy to help proofread and revise grammar as you'd like :-)

keep = self._greaterOrEqual(np.random.uniform(0,1,X.shape))
x = X * keep + alpha_p * (1 - keep)
X = a * x + b
self._wrapper_derived_variables["AlphaDropout_mask"] = keep
Copy link
Owner

Choose a reason for hiding this comment

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

This is inconsistent with dropout_mask in line 165. To maintain consistency with the regular Dropout wrapper, I'd recommend changing this to dropout_mask as well.

self._wrapper_derived_variables["AlphaDropout_mask"] = keep
return scaler * self._base_layer.forward(X)

def backward(self, dLdy):
Copy link
Owner

Choose a reason for hiding this comment

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

This is not correct for AlphaDropout, as X is not scaled by 1 / 1-p - it's a function of alpha, scale, and p now.

@ddbourgin
Copy link
Owner

Hi @BoltzmannZhaung - sorry for the delay. I just did a review pass and left some specific in-line comments. In addition to these, there are two general points I want to emphasize:

  1. Please double check to make sure your code runs when you submit. I noticed at least one bug in your code that would easily have been caught if you tried running it on an example.

  2. It's important that the code in this repo represent a sincere effort at original work. In particular, this means that:

    • It is not directly copied from other sources
    • If it is based off of an existing implementation, those implementations are cited in both the function documentation and the PR itself.

I recognize that for simple functions it can be difficult to implement them cleanly in a way that doesn't resemble existing implementations. This is okay, and I don't expect you to be able to write 100% unique code. What I want, however, is that your code represent your efforts, rather than someone else's :)

@BoltzmannZhaung
Copy link
Author

@ddbourgin
Thanks for ur review and comments! I will rewrite this method with the individual code immediately that represents my individual efforts. This repo which represents a sincere effort at the original work you have declared did inspire me with determination.
THANK YOU!
ps: uh... I give you a sincere apology for my previous bad work. for this time, I will try my best to finish it correctly and as original as possible.
oh~ also as soon as possible.

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.

3 participants