GSOC'16 - DNN modern #720

edgarriba · 2016-06-27T23:56:19Z

This pull request creates a new module for deep learning.

it requires tiny-dnn (check README for installation)
the module provides a caffe model loader
a quick sample on how to use caffe loader is also provided

alalek · 2016-10-12T10:57:34Z

modules/dnn_modern/CMakeLists.txt

+# MODULE REQUIREMENTS
+# ----------------------------------------------------------------------------
+
+find_package(TinyDNN)


please use QUIET mode

alalek · 2016-10-12T11:02:47Z

modules/dnn_modern/cmake/FindTBB.cmake

@@ -0,0 +1,283 @@
+# Locate Intel Threading Building Blocks include paths and libraries


OpenCV has TBB support: https://github.com/opencv/opencv/blob/master/cmake/OpenCVDetectTBB.cmake
What kind of problems do you have with reusing of mentioned code?

the motivation for this is that I want the user to choose if he want to build the module with TBB totally independent of the rest of the opencv modules. So, if opencv was built with TBB support maybe for X reason I don't want to use TBB with this module. /cc @bhack

This is to avoid potential conflicts, which are hard to debug.

my solution or your solution?

alalek · 2016-10-12T11:24:20Z

modules/dnn_modern/cmake/FindTinyCNN.cmake

@@ -0,0 +1,46 @@
+# Locate the tiny-cnn library.


Replace everywhere:

tiny-cnn -> tiny-dnn FindTinyCNN -> FindTinyDNN TinyCNN -> TinyDNN etc

alalek · 2016-10-12T12:10:31Z

modules/dnn_modern/README.md

+  Download tiny-cnn project somewhere in your system
+
+    cd /opt
+    git clone https://github.com/nyanp/tiny-cnn.git


Project has new link:

https://github.com/tiny-dnn/tiny-dnn

Problem is still here.

edgarriba · 2016-10-13T07:49:29Z

@vpisarev @garybradski Are we going to merge this module as it is or do we wait for a clear roadmap for the dnn module? /cc @bhack

bhack · 2016-12-02T13:07:29Z

@edgarriba We are on this topic. I hope that we can have news soon

sovrasov · 2016-12-13T09:19:46Z

@edgarriba this PR will be merged but first you need to resolve infrastructure problems mentioned by @alalek

edgarriba · 2016-12-18T17:25:10Z

@alalek @sovrasov ready to merge

sovrasov · 2016-12-19T13:50:34Z

modules/dnn_modern/CMakeLists.txt

+# ----------------------------------------------------------------------------
+# MODULE REQUIREMENTS
+# ----------------------------------------------------------------------------
+


Add cmake_policy(SET CMP0028 OLD) to avoid warning caused by :: in TinyDNN::tiny_dnn target.

sovrasov · 2016-12-19T14:07:01Z

modules/dnn_modern/CMakeLists.txt

+endif()
+
+find_package(Protobuf)
+find_package(Caffe)


Is Caffe required by this module?

let me check

sovrasov · 2016-12-19T14:12:49Z

modules/dnn_modern/CMakeLists.txt

+# ----------------------------------------------------------------------------
+
+ocv_define_module(dnn_modern opencv_core opencv_imgproc opencv_imgcodecs WRAP python)
+ocv_target_link_libraries(${the_module} TinyDNN::tiny_dnn ${REQUIRED_LIBRARIES})


I followed the instructions form README, but tiny-dnn headers are missing during compilation,
so it's needed to add implicit include of sources.
Also in case if headers are included compilation fails due to -Werror=non-virtual-dtor flag.

Did you download the last version and installed via cmake commands?

Install target in tiny-dnn's CMake is broken at the moment (headers of 3rd party libs are not installed), so I'm just using instructions from readme. CMake command is cmake . -DTINYDNN_ROOT=~/repositories/tiny-dnn/

it's what I suspected. The idea for third-party libraries is that you don't need them to run the internal algorithms. However, it's clear that something is not doing the job. We need to checkout this issue and update the upstream since I don't have a quick answer for this now. /cc @nyanp

sovrasov · 2016-12-19T14:13:41Z

modules/dnn_modern/src/caffe_converter.cpp

+    results->clear();
+    results->reserve(result.size());
+
+    for (cnn_size_t i = 0; i < result.size(); i++) {


cnn_size_t doesn't exist anymore and should be replaced with size_t.

edgarriba · 2017-01-02T12:17:34Z

@sovrasov @alalek could you check again? We fixed some issues in master that allows to compile without external dependencies. I tried in local and it works for me.

sovrasov · 2017-01-09T14:15:32Z

modules/dnn_modern/CMakeLists.txt

@@ -0,0 +1,114 @@
+set(the_description "Modern Deep Learning module")
+


Please add CMake version check here:

if(${CMAKE_VERSION} VERSION_LESS 3.2) message(STATUS "Module opencv_dnn_modern disabled beacuse CMake version is less than 3.2") ocv_module_disable(dnn_modern) return() endif()

sovrasov · 2017-01-09T14:22:03Z

modules/dnn_modern/CMakeLists.txt

+# ----------------------------------------------------------------------------
+
+ocv_define_module(dnn_modern opencv_core opencv_imgproc opencv_imgcodecs WRAP python)
+ocv_target_link_libraries(${the_module} ${REQUIRED_LIBRARIES})


For me it doesn't work unless I add the below two lines:

ocv_target_include_directories(${the_module} ${TINYDNN_INCLUDE_DIRS}) target_compile_options(${the_module} PRIVATE "-Wno-error=non-virtual-dtor")

sovrasov · 2017-01-09T14:32:25Z

modules/dnn_modern/src/caffe_converter.cpp

+
+#define CNN_USE_CAFFE_CONVERTER
+#include <tiny_dnn/tiny_dnn.h>
+


Build fails with tons of linker errors related to caffe, but it can be fixed by adding here
#include <tiny_dnn/io/caffe/caffe.pb.cc>

@sovrasov ok. However, not sure what's the proper way to handle this since in the library those proto files are generated during the cmake configuration
https://github.com/tiny-dnn/tiny-dnn/blob/master/examples/caffe_converter/CMakeLists.txt

Should we include this piece of code here?

I think it's better to handle this in CMake and just didn't know how it done in tiny-dnn.

In case if tiny-dnn was configured with it's own CMake the most part of dnn_modern CMake becomes useless and can be removed, but current implementation can work with raw unconfigured sources. Why did you choose this way instead of relying on CMake from tiny-dnn?

well the point is that tiny-dnn is header-only and very configurable so you have to setup the configuration at the moment you "link" it to the target project. However, if you have any better idea it will be welcomed.

I see your point, just interested why tinydnn-config.cmake is not used.
Devs of tiny-dnn recommend it as far as I can see:

# Example to find TinyDNN headers only:: # # find_package(TinyDNN 0.1.0) # if(TinyDNN_FOUND) # add_executable(foo foo.cc) # target_link_libraries(foo TinyDNN::tiny_cnn) # endif() # # Example to find TinyDNN headers and some *static* libraries:: # # set(TinyDNN_USE_TBB ON) # only find static libs # set(TInyCNN_USE_AVX2 ON) # find_package(TinyDNN 0.1.0) # if(TinyDNN_FOUND) # add_executable(foo foo.cc) # target_link_libraries(foo TinyDNN::tiny_cnn ${TinyDNN_LIBRARIES}) # endif()

well yes, I'm the one who wrote those codes. But to use you need to install tiny-dnn headers via cmake in order to create all the targets. I didn't test at all the config file, so this can be a good starting point.

Let's leave all as is, because now rewriting dnn_modern CMake and testing tiny-dnn config will take too much effort.

add FindTinyCNN.cmake and setup CMakeLists.txt add find TBB and NNPACK to CMakeLists.txt add caffe converter skeleton add simple sample eval method return scores Update README.md Update README.md Update README.md

sovrasov

👍

alalek · 2017-01-17T10:00:36Z

modules/dnn_modern/CMakeLists.txt

+    add_definitions(-DCNN_USE_OMP)
+    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OpenMP_C_FLAGS}")
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
+endif()


OpenCV already defines TBB and OpenMP dependencies.

edgarriba · 2017-01-27T10:23:19Z

@alalek ping

alalek · 2017-02-08T10:26:55Z

@edgarriba
There are still several places with legacy name "tiny-cnn". Could you please replace it to "tiny-dnn" to prevent confusing of developers/users?

edgarriba · 2017-02-08T10:41:37Z

@alalek now should be fine

alalek · 2017-02-08T13:39:25Z

Thanks!

…cv#720)

edgarriba force-pushed the dnn-modern branch 4 times, most recently from 1d8219c to 971f2a7 Compare June 28, 2016 11:10

edgarriba force-pushed the dnn-modern branch 2 times, most recently from 28c53bf to bcfb93e Compare September 11, 2016 11:01

alalek reviewed Oct 12, 2016

View reviewed changes

Auron-X mentioned this pull request Nov 18, 2016

GOTURN Tracker #718

Closed

edgarriba force-pushed the dnn-modern branch from bcfb93e to 66bd965 Compare December 18, 2016 10:45

sovrasov reviewed Dec 19, 2016

View reviewed changes

sovrasov reviewed Jan 9, 2017

View reviewed changes

edgarriba added 7 commits January 13, 2017 21:38

dnn_modern module sketelon

170edd4

add FindTinyCNN.cmake and setup CMakeLists.txt add find TBB and NNPACK to CMakeLists.txt add caffe converter skeleton add simple sample eval method return scores Update README.md Update README.md Update README.md

update for tiny-dnn package

1db48a2

add QUIET attribute to CMake to find tiny-dnn

ebed6c4

update files due to library rename

7a476d0

disable warning in cmake

ec7f703

replace cnn_size_t -> size_t

085a8c2

disable serializer, remove Caffe dependency and remove tiny-dnn target

0ea7094

edgarriba force-pushed the dnn-modern branch from 53cc81e to 0ea7094 Compare January 13, 2017 20:38

edgarriba added 2 commits January 13, 2017 21:49

check CMake version to enable/disable the module

aa7731d

add -Wno-erro in module target

d6c1a48

sovrasov approved these changes Jan 16, 2017

View reviewed changes

alalek reviewed Jan 17, 2017

View reviewed changes

edgarriba added 6 commits January 17, 2017 22:17

update README.md with tiny-dnn

5dcbb23

remove redunadant CMake codes for TBB and OMP

da4ac1a

update CMake to enable C++11 compiler

19cac9d

sync CMake compiler options with upstream project

dac97e5

remove unused FindTBB.cmake

f2cdce0

minor fix to add const variables

5d6c6c5

tiny-cnn -> tiny-dnn

135b568

edgarriba force-pushed the dnn-modern branch from bc8bdd2 to 135b568 Compare February 8, 2017 10:41

alalek assigned sovrasov Feb 8, 2017

alalek merged commit 1a26f99 into opencv:master Feb 8, 2017

allnes pushed a commit to allnes/opencv_contrib that referenced this pull request Jun 9, 2024

Added openvino_contrib-vscode-extension-maintainters codeowners (open…

79dc880

…cv#720)

		@@ -0,0 +1,283 @@
		# Locate Intel Threading Building Blocks include paths and libraries

		@@ -0,0 +1,114 @@
		set(the_description "Modern Deep Learning module")


		#define CNN_USE_CAFFE_CONVERTER
		#include <tiny_dnn/tiny_dnn.h>

GSOC'16 - DNN modern #720

GSOC'16 - DNN modern #720

Uh oh!

Conversation

edgarriba commented Jun 27, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alalek Oct 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edgarriba Dec 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edgarriba commented Oct 13, 2016

Uh oh!

bhack commented Dec 2, 2016

Uh oh!

sovrasov commented Dec 13, 2016

Uh oh!

edgarriba commented Dec 18, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sovrasov Dec 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edgarriba commented Jan 2, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sovrasov Jan 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sovrasov Jan 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sovrasov left a comment

edgarriba commented Jun 27, 2016 •

edited

Loading

alalek Oct 12, 2016 •

edited

Loading

edgarriba Dec 18, 2016 •

edited

Loading

sovrasov Dec 19, 2016 •

edited

Loading

sovrasov Jan 11, 2017 •

edited

Loading

sovrasov Jan 11, 2017 •

edited

Loading

alalek commented Feb 8, 2017 •

edited

Loading