De-duplicate Engine Tests

[shelhamer]

A not insignificant amount of the cuDNN integration diff was testing, but these tests are mostly mirrors of the Caffe standard tests with the engine swapped between CAFFE and CUDNN. These should be de-duplicated by a TestDtypesAndDevicesAndEngines scheme or simply a vector of the two layer objects for clarity and ease of maintenance.

[kloudkl]

The increased test time caused by the added (exploding) combination is perhaps not a big deal. But the smell of the code suggests that refactoring is in need. If completed by someone, #610 and #1064 may provide a more thorough solution.

There are two or three variants, i.e. CPU, CUDA and cuDNN, with the same functionalities for some layers. There will certainly be more in the future. To be flexible and easily extensible, the implementations of the same layer should be split into separate classes inheriting a common abstract base class. For example, AbstractConvolutionLayer is inherited by CPUConvolutionLayer, CUDAConvolutionLayer and CuDNNConvolutionLayer. The modularity of the layers can also be achieved by other designs.

Each family of layers could be produced by the corresponding CPU, CUDA and CuDNN LayerFactory. There are layers that only have CPU version or CPU plus CUDA version. To avoid having to manually track which layer have what versions which is cumbersome, the CuDNNLayerFactory had better subclass CUDALayerFactory which then inherits CPULayerFactory. The subclasses only override the methods to produce the layers that are implemented in the specific flavors.

[shelhamer]

While the full abstract class and inherited strategies design has its
merits, and is actually how I first developed the engines, it was rejected
because

• engines are not completely exchangeable, and a particular implementation
  might lack support for a given configuration e.g. cuDNN can't pool with
  padding
• by inheriting from the standard Caffe implementation, and always setting
  it up, the engine call fallback to Caffe as done by cuDNN layers in CPU
  mode.

I could imagine a factory and strategy design exists to handle this, but
deferred it for this practicality. Good to keep in mind improved designs
all the same.

For the tests, de-duplication could come through pulling the test method
bodies out of the test classes and giving each a Layer arg for passing in
the different engine (sub)classes.

On Saturday, September 20, 2014, kloudkl notifications@github.com wrote:

The increased test time caused by the added (exploding) combination is
perhaps not a big deal. But the smell of the code suggests that refactoring
is in need. If completed by someone, #610
#610 and #1064
#1064 may provide a more thorough
solution.

There are two or three variants, i.e. CPU, CUDA and cuDNN, with the same
functionalities for some layers. There will certainly be more in the
future. To be flexible and easily extensible, the implementations of the
same layer should be split into separate classes inheriting a common
abstract base class. For example, AbstractConvolutionLayer is inherited by
CPUConvolutionLayer, CUDAConvolutionLayer and CuDNNConvolutionLayer. The
modularity of the layers can also be achieved by other designs.

Each family of layers could be produced by the corresponding CPU, CUDA and
CuDNN LayerFactory. There are layers that only have CPU version or CPU plus
CUDA version. To avoid having to manually track which layer have what
versions which is cumbersome, the CuDNNLayerFactory had better subclass
CUDALayerFactory which then inherits CPULayerFactory. The subclasses only
override the methods to produce the layers that are implemented in the
specific flavors.

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

[rddesmond]

This is related to the issues that are brought up in "Set mode cpu fails?" #3317 (though, this of course, predates that by a lot). It's a shame that "Device Abstraction" #610 was abandoned, because it would resolve a lot of this.