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

C-LSTM with mask #6316

Closed
liweigu opened this Issue Aug 30, 2018 · 16 comments

Comments

Projects
None yet
3 participants
@liweigu
Copy link

liweigu commented Aug 30, 2018

I'm training a model to use time-serial images to predict new images, using convolutional LSTM, set RnnToCnnPreProcessor to the first layer(ConvolutionLayer). In order to use n images to predict new n images, i'm using the 'many-to-many', set feature and label both are of (n x 2) size, and set featuresMask as [1,1..,1,0,...,0], set labelsMask as [0,0,...,0,1,...,1]. But it got error :
'Expected rank 4 mask array for 2D CNN layers. Mask arrays for 2D CNN layers must have shape [batchSize,channels,X,Y]'.
It seems the featuresMask is applied to input as CNN, how to apply it to input as RNN?

As my raw code is a bit confused, i re-organized the code at
https://gist.github.com/liweigu/f0dafd8045d766b39c23e1b52938b2ac
(Assuming minibatch = 1, the code is simplied. I can add more code if needed.)

@AlexDBlack AlexDBlack self-assigned this Aug 30, 2018

@AlexDBlack

This comment has been minimized.

Copy link
Member

AlexDBlack commented Aug 30, 2018

Thanks for the code, I was able to reproduce the issue quickly.

Looks like this is a bug - a release or two back we switched the CNN masking from 2d to broadcastable 4d, to avoid ambiguity and allow greater flexibility with the type of masking for CNNs.
However, this wasn't updated in the CNN to/from RNN preprocesors (looks like they just got forgotten).
I'm working on a fix now.

@AlexDBlack

This comment has been minimized.

Copy link
Member

AlexDBlack commented Aug 30, 2018

Fixed here: #6319
Just waiting on full test run before merging.

Edit: oh, and I noticed your masks were around the wrong way. Minibatch always comes first, sequence length second.

AlexDBlack added a commit that referenced this issue Aug 30, 2018

#6316 Fix CNN/RNN masking (#6319)
* #6316 Fix CNN/RNN masking

* Minor comment fix
@liweigu

This comment has been minimized.

Copy link

liweigu commented Aug 30, 2018

Thank you!
I'll re-test it then.

@liweigu

This comment has been minimized.

Copy link

liweigu commented Sep 3, 2018

I'm still getting error, please check it at:
https://gist.github.com/liweigu/8af223c3e245775d11f276cfe26e28ef

@AlexDBlack

This comment has been minimized.

Copy link
Member

AlexDBlack commented Sep 3, 2018

@liweigu Did you upgrade to snapshots first, and build with mvn -U option to ensure you got the latest snapshots?
If you're still on 1.0.0-beta2, then of course you won't have the fix...

@liweigu

This comment has been minimized.

Copy link

liweigu commented Sep 3, 2018

I'm using snapshots and I've upgraded it with mvn -U option.

@AlexDBlack

This comment has been minimized.

Copy link
Member

AlexDBlack commented Sep 3, 2018

OK, I'll re-run your code later and see.
I did add a unit test derived from your code though, when implementing the fix.

@AlexDBlack AlexDBlack reopened this Sep 3, 2018

@liweigu

This comment has been minimized.

Copy link

liweigu commented Sep 3, 2018

Here's the full code for getComputationGraph():
https://gist.github.com/liweigu/2e240e3532a8478080f291d9e370efa7

@AlexDBlack

This comment has been minimized.

Copy link
Member

AlexDBlack commented Sep 3, 2018

I'm not able to reproduce this. Here's the exact code I ran, derived from your code: https://gist.github.com/AlexDBlack/916dc4dd2a3df5d5b3241a0f60ff2285

I'm going to close this - it's probably still outdated snapshots on your end. Or, perhaps invalid input.

If you can provide me with something complete I can run without any modification or addition to reproduce this - then I'm happy to look into this more.

@AlexDBlack AlexDBlack closed this Sep 3, 2018

@liweigu

This comment has been minimized.

Copy link

liweigu commented Sep 4, 2018

The jars in the snapshot are just updated to 20180828:
https://oss.sonatype.org/content/repositories/snapshots/org/deeplearning4j/deeplearning4j-nn/1.0.0-SNAPSHOT/
I think that's why i cann't get the fresh jars.
Maybe something wrong in daily build.

@AlexDBlack

This comment has been minimized.

Copy link
Member

AlexDBlack commented Sep 4, 2018

That probably explains it then... cc @sshepel

@sshepel

This comment has been minimized.

Copy link
Contributor

sshepel commented Sep 4, 2018

@liweigu sorry for inconvenience, working on that, hope fresh snapshots will be uploaded today.

@liweigu

This comment has been minimized.

Copy link

liweigu commented Sep 4, 2018

Thanks ~

@sshepel

This comment has been minimized.

Copy link
Contributor

sshepel commented Sep 5, 2018

@liweigu we got successful build for master, could you please check snapshots repo for new artifacts.

@liweigu

This comment has been minimized.

Copy link

liweigu commented Sep 7, 2018

It was fine thereafter.
Just see your message, so i reply this now.
Thanks again.

@lock

This comment has been minimized.

Copy link

lock bot commented Oct 7, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Oct 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.