removed 4dimensions constraints on blob shape

[albenoit]

When trying to setup conv layer and specifying kernel size this way (see the complete conv layer at the end) :

kernel_size: [3, 3, 11]

Current master code returns error

F1112 10:02:11.418356 32749 blob.hpp:140] Check failed: num_axes() <= 4 (5 vs. 4) Cannot use legacy accessors on Blobs with > 4 axes.

I simply removed the 4 dimension constraints following pull requests on the topic #2049

It seems to work now, tell me if something else should be considered.

For testing, here is the beginning of the architecture and caffe log

layer {
  type: "DummyData"
  top: "data"
  top: "label"
  dummy_data_param {
    shape { dim: 10 dim:1 dim: 3 dim: 3 dim: 103 }
    shape { dim: 10 }
  }
}
layer {
          name: "conv1"
          type: "Convolution"
          bottom: "data"
          top: "conv1"
          param {
              name: "conv1_w"
              lr_mult:    1
              decay_mult: 1    
          }
          param {
              name: "conv1_b"
              lr_mult:    2    
              decay_mult: 0    
          }
          convolution_param {
              num_output: 20
              kernel_size: [3, 3, 11]
              stride: 1
              pad: 0
                 weight_filler{
                      type: "xavier"
                }
              bias_filler {
                    type: "constant" 
                    value: 0
              }
         }
}

related log:

I1112 10:22:45.839669 22068 net.cpp:106] Creating Layer 
I1112 10:22:45.839684 22068 net.cpp:411]  -> data
I1112 10:22:45.839702 22068 net.cpp:411]  -> label
I1112 10:22:45.839738 22068 net.cpp:150] Setting up 
I1112 10:22:45.839753 22068 net.cpp:157] Top shape: 10 1 3 3 103 (9270)
I1112 10:22:45.839767 22068 net.cpp:157] Top shape: 10 (10)
I1112 10:22:45.839777 22068 net.cpp:165] Memory required for data: 37120
I1112 10:22:45.839788 22068 layer_factory.hpp:76] Creating layer label__1_split
I1112 10:22:45.839803 22068 net.cpp:106] Creating Layer label__1_split
I1112 10:22:45.839817 22068 net.cpp:454] label__1_split <- label
I1112 10:22:45.839833 22068 net.cpp:411] label__1_split -> label__1_split_0
I1112 10:22:45.839848 22068 net.cpp:411] label__1_split -> label__1_split_1
I1112 10:22:45.839865 22068 net.cpp:150] Setting up label__1_split
I1112 10:22:45.839880 22068 net.cpp:157] Top shape: 10 (10)
I1112 10:22:45.839892 22068 net.cpp:157] Top shape: 10 (10)
I1112 10:22:45.839903 22068 net.cpp:165] Memory required for data: 37200
I1112 10:22:45.839915 22068 layer_factory.hpp:76] Creating layer conv1
I1112 10:22:45.839932 22068 net.cpp:106] Creating Layer conv1
I1112 10:22:45.839943 22068 net.cpp:454] conv1 <- data
I1112 10:22:45.839961 22068 net.cpp:411] conv1 -> conv1
I1112 10:22:45.840006 22068 net.cpp:150] Setting up conv1
I1112 10:22:45.840020 22068 net.cpp:157] Top shape: 10 20 1 1 93 (18600)

[seanbell]

Once you are outside 4D, it doesn't make sense to use the legacy accessors num, channels, height, width, since they are defined with respect to a specific convention that only applies in 4D. I think the solution is to update the code that uses the legacy methods to use index numbers (e.g. axis 0 instead of num), and not to do what this PR is suggesting.

I think instead you should find the part of the code that used one of the legacy accessors and upgrade it to use the ND accessor.

[seanbell]

See #2049 for info about ND convolution.

[albenoit]

Hi,
thank you for the feedback.
Well, if i understand well the different ideas discusses along pull requests and particularly this one:
#2959
it seems that yes, specifying a list to a single kernel_size parameter i a single line is no more appropriate.
One should only have one value per kernel_size parameter, one for each dimension.

However, all those changes are pretty recent. Then, should one definitely close this PR or are there any pending discussions on the topic ?

Actually one should decide on the syntax and take this into account in the python drawing script caffe.draw.draw_net_to_file :
_if providing a list of kernel sizes, caffe.draw.draw_net_to_file fails to draw since it does not support lists
_if providing one kernel_size parameter line per dimension, caffe.draw.draw_net_to_file only shows the first one on the generated schema.
thanks

[albenoit]

Hi again,
i tested the approach 3D convolution on the master branch using the last discussed convention for kernel params specs (one line per dimension in the prototxt file).
Unfortunately, we still got the error cited at the beginning of this PR:

F1112 10:02:11.418356 32749 blob.hpp:140] Check failed: num_axes() <= 4 (5 vs. 4) Cannot use legacy accessors on Blobs with > 4 axes.

The only solution was to apply the PR changes to make it work.
Did i do something wrong ? Here is the net spec used:

          convolution_param {
              # learn 20 filters
              num_output: 20
              #kernel_size: (1, 1, 5)
              kernel_size: 1
              kernel_size: 1
              kernel_size: 5
              # step 1 pixels between each filter application
              stride: 1
              # padding
              pad: 0
              # initialize the filters from given distribution model

                 weight_filler{
                      type: "xavier"
                }

[jeffdonahue]

The problem, as @seanbell suggested, is likely that there is another layer (not ConvolutionLayer) in your network that uses one or more of the legacy dimension calls (num(), channels(), height(), or width()) and as such doesn't know how to handle blobs with more than 4 dimensions.

In general, every CHECK in the codebase is there because proceeding without the assumption enforced by said CHECK likely leads to incorrect behavior, and advice to remove or comment out these checks from people who aren't sure why they're there is almost always misguided. I'm closing this because there might be particular places where it happens that these legacy dimension checks are safe to ignore but in general it's definitely not safe to remove them.