Add ceil_mode to max pooling and average pooling

[hzxie]

See PR #3057 in Caffe.

However, I cannot build tests and examples.
I think it is caused by something defined in caffe.proto.

I've added following line into caffe.proto, but still not working.

optional bool ceil_mode = 13 [default = false];

And here's the error message:

In file included from /usr/lib/gcc/x86_64-pc-linux-gnu/7.3.1/include/c++/x86_64-pc-linux-gnu/bits/c++allocator.h:33:0,
                 from /usr/lib/gcc/x86_64-pc-linux-gnu/7.3.1/include/c++/bits/allocator.h:46,
                 from /usr/lib/gcc/x86_64-pc-linux-gnu/7.3.1/include/c++/string:41,
                 from /home/hzxie/Development/tiny-dnn/test/cotire/tiny_dnn_test_CXX_prefix.cxx:6,
                 from /home/hzxie/Development/tiny-dnn/test/cotire/tiny_dnn_test_CXX_prefix.hxx:4:
/usr/lib/gcc/x86_64-pc-linux-gnu/7.3.1/include/c++/ext/new_allocator.h: In instantiation of ‘void __gnu_cxx::new_allocator<_Tp>::construct(_Up*, _Args&& ...) [with _Up = tiny_dnn::max_pooling_layer; _Args = {const long unsigned int&, const long unsigned int&, const long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, tiny_dnn::padding&}; _Tp = tiny_dnn::max_pooling_layer]’:
/usr/lib/gcc/x86_64-pc-linux-gnu/7.3.1/include/c++/bits/alloc_traits.h:475:4:   required from ‘static void std::allocator_traits<std::allocator<_CharT> >::construct(std::allocator_traits<std::allocator<_CharT> >::allocator_type&, _Up*, _Args&& ...) [with _Up = tiny_dnn::max_pooling_layer; _Args = {const long unsigned int&, const long unsigned int&, const long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, tiny_dnn::padding&}; _Tp = tiny_dnn::max_pooling_layer; std::allocator_traits<std::allocator<_CharT> >::allocator_type = std::allocator<tiny_dnn::max_pooling_layer>]’
/usr/lib/gcc/x86_64-pc-linux-gnu/7.3.1/include/c++/bits/shared_ptr_base.h:526:39:   required from ‘std::_Sp_counted_ptr_inplace<_Tp, _Alloc, _Lp>::_Sp_counted_ptr_inplace(_Alloc, _Args&& ...) [with _Args = {const long unsigned int&, const long unsigned int&, const long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, tiny_dnn::padding&}; _Tp = tiny_dnn::max_pooling_layer; _Alloc = std::allocator<tiny_dnn::max_pooling_layer>; __gnu_cxx::_Lock_policy _Lp = (__gnu_cxx::_Lock_policy)2]’
/usr/lib/gcc/x86_64-pc-linux-gnu/7.3.1/include/c++/bits/shared_ptr_base.h:637:4:   required from ‘std::__shared_count<_Lp>::__shared_count(std::_Sp_make_shared_tag, _Tp*, const _Alloc&, _Args&& ...) [with _Tp = tiny_dnn::max_pooling_layer; _Alloc = std::allocator<tiny_dnn::max_pooling_layer>; _Args = {const long unsigned int&, const long unsigned int&, const long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, tiny_dnn::padding&}; __gnu_cxx::_Lock_policy _Lp = (__gnu_cxx::_Lock_policy)2]’
/usr/lib/gcc/x86_64-pc-linux-gnu/7.3.1/include/c++/bits/shared_ptr_base.h:1295:35:   required from ‘std::__shared_ptr<_Tp, _Lp>::__shared_ptr(std::_Sp_make_shared_tag, const _Alloc&, _Args&& ...) [with _Alloc = std::allocator<tiny_dnn::max_pooling_layer>; _Args = {const long unsigned int&, const long unsigned int&, const long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, tiny_dnn::padding&}; _Tp = tiny_dnn::max_pooling_layer; __gnu_cxx::_Lock_policy _Lp = (__gnu_cxx::_Lock_policy)2]’
/usr/lib/gcc/x86_64-pc-linux-gnu/7.3.1/include/c++/bits/shared_ptr.h:344:64:   required from ‘std::shared_ptr<_Tp>::shared_ptr(std::_Sp_make_shared_tag, const _Alloc&, _Args&& ...) [with _Alloc = std::allocator<tiny_dnn::max_pooling_layer>; _Args = {const long unsigned int&, const long unsigned int&, const long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, tiny_dnn::padding&}; _Tp = tiny_dnn::max_pooling_layer]’
/usr/lib/gcc/x86_64-pc-linux-gnu/7.3.1/include/c++/bits/shared_ptr.h:690:14:   required from ‘std::shared_ptr<_Tp> std::allocate_shared(const _Alloc&, _Args&& ...) [with _Tp = tiny_dnn::max_pooling_layer; _Alloc = std::allocator<tiny_dnn::max_pooling_layer>; _Args = {const long unsigned int&, const long unsigned int&, const long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, tiny_dnn::padding&}]’
/usr/lib/gcc/x86_64-pc-linux-gnu/7.3.1/include/c++/bits/shared_ptr.h:706:39:   required from ‘std::shared_ptr<_Tp> std::make_shared(_Args&& ...) [with _Tp = tiny_dnn::max_pooling_layer; _Args = {const long unsigned int&, const long unsigned int&, const long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, tiny_dnn::padding&}]’
/home/hzxie/Development/tiny-dnn/tiny_dnn/io/caffe/layer_factory_impl.h:139:46:   required from here
/usr/lib/gcc/x86_64-pc-linux-gnu/7.3.1/include/c++/ext/new_allocator.h:136:4: error: no matching function for call to ‘tiny_dnn::max_pooling_layer::max_pooling_layer(const long unsigned int&, const long unsigned int&, const long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, long unsigned int&, tiny_dnn::padding&)’
  { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); }

What should I do to fix this error?

[hzxie]

The compile error can be resolved by adding two constructors (that make API unchanged):

max_pooling_layer(size_t in_width,
                    size_t in_height,
                    size_t in_channels,
                    size_t pooling_size_x,
                    size_t pooling_size_y,
                    size_t stride_x,
                    size_t stride_y,
                    padding pad_type             = padding::valid,
                    core::backend_t backend_type = core::default_engine())
    : max_pooling_layer(in_width,
                        in_height,
                        in_channels,
                        pooling_size_x,
                        pooling_size_y,
                        stride_x,
                        stride_y,
                        false,
                        padding::valid,
                        backend_type) {}

and

average_pooling_layer(size_t in_width,
                        size_t in_height,
                        size_t in_channels,
                        size_t pool_size_x,
                        size_t pool_size_y,
                        size_t stride_x,
                        size_t stride_y,
                        padding pad_type = padding::valid) 
    : average_pooling_layer(in_width,
                            in_height,
                            in_channels,
                            pool_size_x,
                            pool_size_y,
                            stride_x,
                            stride_y,
                            false,
                            padding::valid) {}

But I don't think it is a good solution.

[hzxie]

@edgarriba Please also review this PR.

[beru]

I've come to conclusion that having a lot of constructor arguments (and overloaded constructors...) leads to dead end.

Specifying layer parameters with string argument is more robust and generic solution and runtime parsing cost should be very negligible compared to actual computations.

C++ Designated initializers can be another solution but we have to wait for C++20.

[hzxie]

@beru So how to solve this problem?
Or remain it unsolved?

[beru]

@hzxie I didn't understand the problem. Let me check how to update tiny_dnn/io/caffe/layer_factory_impl.h.

However, I still think this library's design around having massive amount of overloaded constructors hinders maintainability...

[beru]

I guess create_max_pool and create_ave_pool and create_pooling functions in tiny_dnn/io/caffe/layer_factory_impl.h should be updated to support the new parameter.

[hzxie]

@beru
The problem has been resolved by adding corresponding parameters in tiny_dnn/io/caffe/layer_factory_impl.h.
Please review it again.

[beru]

LGTM!