keras-team / keras

Deep Learning for humans
http://keras.io/
Apache License 2.0
61.85k stars 19.44k forks source link

Interest in adding causal flag to AtrousConvolution1D #5297

Closed basveeling closed 7 years ago

basveeling commented 7 years ago

The authors of the Wavenet paper propose a variation of dilated 1d convolutions for temporal data dubbed causal dilated convolutions, which essentially implies that output[t] relies solely on input from the 'past': input[x:t] x < t.

Is there interest for a PR that adds a causal flag to AtrousConvolution1D which ensures this behavior? It would entail the following changes to AtrousConvolution1D: https://github.com/basveeling/wavenet/blob/master/wavenet_utils.py#L11

fchollet commented 7 years ago

We plan on removing the AtrousConv layers and instead supporting dilation directly in the Conv layers. How do you envision support for causality in this context, API-wise?

On 6 February 2017 at 12:04, Bas Veeling notifications@github.com wrote:

The authors of the Wavenet paper propose a variation of dilated 1d convolutions for temporal data dubbed causal dilated convolutions, which essentially implies that output[t] relies solely on input from the 'past': input[x:t] x < t.

Is there interest for a PR that adds a causal flag to AtrousConvolution1D which ensures this behavior? It would entail the following changes to AtrousConvolution1D: https://github.com/basveeling/ wavenet/blob/master/wavenet_utils.py#L11

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/fchollet/keras/issues/5297, or mute the thread https://github.com/notifications/unsubscribe-auth/AArWb4L51BQtI8CdHfEpxjGQfBt7-34Bks5rZ3zEgaJpZM4L4pyN .

basveeling commented 7 years ago

A simple 'causal' flag for the 1D convolutions would make sense, although I'm on the fence whether it would be used enough to warrant clogging up the 1DConvolution API. Another option would be to create a Causal1DConvolution subclass that has the causal flag and see if it is being used before absorbing it into the main class. Thoughts?

fchollet commented 7 years ago

We can go either way. In any case, a PR would have to wait for the Keras 2 API.

On 6 February 2017 at 13:17, Bas Veeling notifications@github.com wrote:

A simple 'causal' flag for the 1D convolutions would make sense, although I'm on the fence whether it would be used enough to warrant clogging up the 1DConvolution API. Another option would be to create a Causal1DConvolution subclass that has the causal flag and see if it is being used before absorbing it into the main class. Thoughts?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/fchollet/keras/issues/5297#issuecomment-277816065, or mute the thread https://github.com/notifications/unsubscribe-auth/AArWbz2zAz9IzIOwuuxQInoAUEj7BAWGks5rZ43MgaJpZM4L4pyN .

basveeling commented 7 years ago

I wasn't aware of a v2 release, any info on that? I'll keep an eye out for a v2 branch to submit a PR to in that case. Cheers!

fchollet commented 7 years ago

The v2 branch should come out in a few days, with a release with ~3 weeks. It will not introduce much new functionality, mostly refactoring and API changes (while maintaining backwards compatibility).

On 6 February 2017 at 13:29, Bas Veeling notifications@github.com wrote:

I wasn't aware of a v2 release, any info on that? I'll keep an eye out for a v2 branch to submit a PR to in that case. Cheers!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/fchollet/keras/issues/5297#issuecomment-277819583, or mute the thread https://github.com/notifications/unsubscribe-auth/AArWb9kzCd66-NmNPtuNSeBxYYT8G6fjks5rZ5C-gaJpZM4L4pyN .

unrealwill commented 7 years ago

Hello. I'm also using some dilated causal convolutions. I use them so we can see input[x:t] x <= t (Note the = sign We can see input t at time t).

Some food for thoughts : I have written two versions API wise to maximize code reuse: The first one is in the style of residual building blocks. (Currently the one I use)

The other one is in imitation of Layer style (a wrapping of the first one to give standard layer calling style) (But it's not perfect because it needs "input_dim" extra information which could normally be calculated automatically based upon the previous layer, (if model input size are evaluated more lazily and only upon compilation and not on model creation, but it comes with the usual tradeoff (in terms of debug-ability) of delayed computation)).

Below is my code :

def DilatedCausalConvolution( nbfilter, filter_size,dilation, x ):

See keras.utils.np_utils conv_output_length

dilated_filter_size = filter_size + (filter_size - 1) * (dilation - 1)
#output_length = input_length - dilated_filter_size + 1
#The goal is to have output_length = xlength
zeroPad = ZeroPadding1D(padding=(dilated_filter_size-1,0))(x)
out = AtrousConvolution1D( nbfilter, filter_size, atrous_rate=dilation,border_mode="valid" )(zeroPad)
return out

class DilatedCausalConvolutionModel: def init(self,input_dim,nbfilter,filter_size,dilation, kwargs): inp = Input(batch_shape=(None, None, input_dim)) out = DilatedCausalConvolution(nbfilter, filter_size, dilation, inp) self.model = Model(inp, out) def call(self, x, kwargs): return self.model(x)

basveeling commented 7 years ago

Quick update: I've given this a shot, but it looks like the tests are not updated to the new API yet. I'll wait for the v2 release and take a look again.