Skip to content

Conversation

@MinhazPalasara
Copy link

No description provided.

@fchollet
Copy link
Collaborator

Could you add a docstring to the example explaining what it does, how long it takes to train and how well it performs?

Likewise for the dataset, it would require more explicit context.

@MinhazPalasara
Copy link
Author

Added comments in the example and dataset file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Better to put this text in a docstring.

@MinhazPalasara
Copy link
Author

I see that the convolution2D has been upgraded to cuDNN backend. I believe it would be consistent to upgrade the convolution3D too before the merge.

@MinhazPalasara
Copy link
Author

Hi,

I have upgraded the conv3d layer and maxpool 3d to the latest keras
standards. I am currently running network on a sample data. It would be
available in a day or 2.

Thanks,
Minhaz

On Tue, Oct 27, 2015 at 11:00 AM, kleinash [email protected] wrote:

Hi.. is this available?


Reply to this email directly or view it on GitHub
#718 (comment).

@rbharath
Copy link

Does this code offer support for multiple channels? For 2D image data, we often have RGB channels, so the actual size of a 2D, 256 pixel image would be (256, 256, 3). For 3D data, is it possible to have multiple channels for the convolution? For example, if we have RGB channels in 3D, then the actual size of a sample grid would be (32, 32, 32, 3).

@rbharath
Copy link

I just realized that I forgot to thank @MinhazPalasara for the great work in putting together this PR. I've been using it for my research, and it's saved me a tremendous amount of time in getting things off the ground!

To answer my own question above, yes this PR does support multiple 3D channels. However, the required input shape is arcane. The input training data X must be of shape

(n_samples, axis_size, n_channels, axis_size, axis_size)

For comparison, the cifar10 example in examples/cifar10_cnn.py generates training data X of shape

(n_samples, n_channels, axis_size, axis_size)

Could you update Convolution3D to expect an input array of form

(n_samples, n_channels, axis_size, axis_size, axis_size)

to match the other cnn example? This is just a suggestion, and perhaps @fchollet or other keras maintainers would have more insight into the correct input format.

@fchollet
Copy link
Collaborator

Could you update Convolution3D to expect an input array of form (n_samples, n_channels, axis_size, axis_size, axis_size)

That would definitely be necessary, for consistency with the API of Convolution2D and Convolution1D.

There are only two input formats that would make sense:

(n_samples, n_channels, axis_size, axis_size, axis_size)
or
(n_samples, axis_size, axis_size, axis_size, n_channels)

But since we are already using the first format for our other convolution layers, that's a no-brainer.

@MinhazPalasara
Copy link
Author

I took care of the argument order in the latest changes.
Details: Theano has no implementation of conv3d that supports both cpu and gpu. I have used conv3d2d.conv3d for gpu and nnet.conv3d for cpu. Both these implementations have different order of arguments. Following this I do dimension shuffling within the conv3d wrapper of keras, abstracting the difference of order from users.

Additionally, I was hopping to add the dnn conv3d to the functionality, but it seems conv3d in dnn is not exposed to theano as yet. So, I would commit the support for cpu and gpu(without dnn backbone) right away.

Thanks,
Minhaz

@nerduno
Copy link

nerduno commented Oct 28, 2015

@minhaz I get a shape mismatch exception who I use convolution3D layers with border_mode='full'

Have you tested this?

@MinhazPalasara
Copy link
Author

Hi nerduno, the functions I have used in this PR don't support any other mode than valid. I will, in a day, commit new changes with all the modes. That is the only thing left, I am currently testing that. Give me a day.

@mrwns
Copy link

mrwns commented Nov 12, 2015

Hi @MinhazPalasara, do you have any news on your 3d layers? I would love to try (and use) them :)

@rbharath
Copy link

I've been using this PR in my pipeline, and I've discovered (and fixed) a few minor bugs associated with saving and loading Convolution3D layers.

@fchollet, @MinhazPalasara : Is there a standard procedure in place for fixing bugs in an outstanding PR? I could create a new PR based off this one, which could be subsequently closed once the bugfixes are merged into this PR. I'm open to any other suggestions on how to get the bugfixes merged in here.

@MinhazPalasara
Copy link
Author

Sorry guys,
Working on it. I was trying to extend ducnn 3d into theano (which is not available right now). Anyway I will commit changes soon to the branch.

@MinhazPalasara
Copy link
Author

@fchollet this branch is up to date with the new keras standards!! Conv3D is supported in gpu and cpu with all possible border modes.

cudnn 3D (conv and pooling) is not supported by theano yet, will try to expose that first and then will update keras.

While developing the "same" border mode for convolution3D, I found a bug in Convolution2D. For the same mode the size of output image is not same. I am logging an issue for that.

@mdering
Copy link
Contributor

mdering commented Dec 7, 2015

Hey is there any update on this PR?

@evanfeinberg
Copy link

I've been using this PR in my ML pipeline. For the most part it seems to work, though I had to make a few fixes:

  1. Line 10 of keras/utils/layer_utils.py should be replaced with
    from ..layers.convolutional import Convolution1D, Convolution2D, Convolution3D, MaxPooling1D, MaxPooling2D, MaxPooling3D, ZeroPadding2D
  2. In method get_config for Convolution3D, the key stack_size is no longer required and should be removed.
  3. Moreover, the method get_config should also include the following two lines:
base_config = super(Convolution3D, self).get_config()
return dict(list(base_config.items()) + list(config.items()))

to include fields for super class Layer (like Convolution2D does).

It would be awesome, @MinhazPalasara and @fchollet , if this PR could be merged soon. I'm glad to help with testing if it would facilitate the process.

@jbencook
Copy link
Contributor

Yeah +1. This would be very useful and I'm happy to help with testing or docs.

@MinhazPalasara
Copy link
Author

@evanfeinberg I will do changes as per your suggestions shortly. It would be great if you could/point me to some open source 3D data that could be a good example for this PR.

@evanfeinberg
Copy link

Thanks, @jbencook and @MinhazPalasara ! @rbharath , do you have any suggestions for pertinent open source 3D data?

@rbharath
Copy link

I think the modelnet database would be a good open-source example of 3D data.

@jruales
Copy link

jruales commented Jan 8, 2016

To make this work with both backends, it might be possible to rewrite Theano's conv3d2d (https://github.com/Theano/Theano/blob/master/theano/tensor/nnet/conv3d2d.py#L170) using Keras's backend-agnostic operators

@RagMeh11
Copy link

I am not able to find Convolution3D layer in convolution.py file. Also there is no defination in the docs also. How can I use this layer and how can I define it in my CNN architecture?? I am working with Medical Images so for that this layer is necessary

@rbharath
Copy link

You want to check out this PR locally (see https://gist.github.com/piscisaureus/3342247 for a guide on how to do that).

@oeway
Copy link
Contributor

oeway commented Feb 1, 2016

Hi, is there anyone working on improving this PR to the up-to-date api of keras? If not, I would try to work on it.
I just checked, this PR is using api from keras 2.0.

@evanfeinberg
Copy link

Thanks for pointing this out, @oeway , PR #718 is indeed broken for keras head. We would love to see 3D CNN's working natively with the latest version.

@MinhazPalasara
Copy link
Author

@oeway @evanfeinberg, I am working on making this PR up to date. Unfortunately tensor flow and theano (cuDnn 3D is not exposed in theano) have no convolution 3D. I am working on pushing changes in those libraries first. In mean time, I could push 3D support for only theano backend.

@rbharath
Copy link

rbharath commented Feb 2, 2016

@MinhazPalasara (cc @oeway @evanfeinberg): It would be great to get some reasonable implementation of this PR merged into keras as soon as possible without waiting for conv3D support in tensorflow/theano. I personally need some extra features on top of a vanilla 3D CNN, but I'm blocked from adding these features to keras since this PR has been open for so long...

@StephanHeijl
Copy link
Contributor

I'd love to see this PR completed! Correct me if I'm wrong, but according to these Theano docs it looks like Theano does expose some 3DConvolutional methods, at least for 0.7.

@oeway
Copy link
Contributor

oeway commented Feb 2, 2016

I am working on this too, and now I am testing the code, should be ready soon.

At the moment, only theano backend is implemented, if anyone can help on the convolution3d implementation on tensorflow, it would be perfect.

@MinhazPalasara , are you also working on tensorflow? If yes, that would be great, and I could taken care of the updates from the keras side.

Details about convolution3d in theano:

There are two implementations available in theano, I used conv3d2d.conv3d for both GPU and CPU, because it's faster:

 * on CPU:
        *  conv3d2d.conv3d:    160 ms per loop
output:
[  2.30778933   2.12729692  -3.06885958  -4.90572691  10.89875412
   3.7447443   -4.60463953   8.3501606    3.69148254  -0.93439424]

        *  nnet.conv3D: 1.19 s per loop
output:
[  2.30778956   2.12729645  -3.0688591   -4.90572643  10.89875507
   3.74474525  -4.60463953   8.35016346   3.69148254  -0.93439448]

 * on GPU (k40):

        *  conv3d2d.conv3d:    36.2 ms per loop
output:
[  2.30778933   2.12729692  -3.06885958  -4.90572691  10.89875412
   3.7447443   -4.60463953   8.3501606    3.69148254  -0.93439424]

       *  nnet.conv3D: 941 ms per loop
output:
[  2.30778956   2.12729645  -3.0688591   -4.90572643  10.89875507
   3.74474525  -4.60463953   8.35016346   3.69148254  -0.93439448]

Notice that there are precision differences with conv3d2d.conv3d on GPU and CPU. The testing code can be found in this discussion: https://groups.google.com/d/msg/theano-users/1S9_bZgHxVw/0cQR9a4riFUJ.

As you can see, conv3d2d.conv3d seems the best on both GPU and CPU.
@MinhazPalasara why do you choose nnet.conv3D for CPU in your PR?

@oeway
Copy link
Contributor

oeway commented Feb 2, 2016

Hi,
check out PR #1623, it's modified from this PR. Now it should be fully functional with theano backend. I will wait for someone to implement conv3d for tensorflow.

@fchollet fchollet closed this Feb 22, 2016
@jruales
Copy link

jruales commented Feb 23, 2016

@rbharath @nerduno @mrwns @evanfeinberg @jbencook @imraghs @StephanHeijl
Convolution3D layers are now available as part of Keras (for the Theano backend).

See PR #1623 for more information.

@jbencook
Copy link
Contributor

👍 thanks!

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.