Skip to content

dnn: split dnn.cpp code#21662

Merged
opencv-pushbot merged 2 commits intoopencv:4.xfrom
alalek:dnn_split
Mar 17, 2022
Merged

dnn: split dnn.cpp code#21662
opencv-pushbot merged 2 commits intoopencv:4.xfrom
alalek:dnn_split

Conversation

@alalek
Copy link
Copy Markdown
Member

@alalek alalek commented Feb 26, 2022

  • extract Net::Impl interface into header (future PRs will add inheritance and plugin support)
  • Net methods implementation are just wrappers over Net::Impl methods without any logic inside

TODO:

  • add dnn.cpp stub with information about previous version (git history is unable to track massive changes)

Validate:

  • OpenVINO
  • Halide backend
  • Vulkan backend
  • CUDA backend
Details
 26 files changed, 6750 insertions(+), 5857 deletions(-)
 delete mode 100644 modules/dnn/src/dnn.cpp
 create mode 100644 modules/dnn/src/dnn_params.cpp
 create mode 100644 modules/dnn/src/dnn_read.cpp
 create mode 100644 modules/dnn/src/dnn_utils.cpp
 create mode 100644 modules/dnn/src/layer.cpp
 create mode 100644 modules/dnn/src/layer_factory.cpp
 create mode 100644 modules/dnn/src/layer_internals.hpp
 create mode 100644 modules/dnn/src/legacy_backend.cpp
 create mode 100644 modules/dnn/src/legacy_backend.hpp
 create mode 100644 modules/dnn/src/net.cpp
 create mode 100644 modules/dnn/src/net_impl.cpp
 create mode 100644 modules/dnn/src/net_impl.hpp
 create mode 100644 modules/dnn/src/net_impl_backend.cpp
 create mode 100644 modules/dnn/src/net_impl_fuse.cpp
 create mode 100644 modules/dnn/src/net_openvino.cpp
 create mode 100644 modules/dnn/src/net_quantization.cpp
 create mode 100644 modules/dnn/src/op_cuda.cpp
 create mode 100644 modules/dnn/src/registry.cpp
force_builders=Custom,Custom Win
build_image:Custom Win=openvino-2021.4.2

Xbuild_image:Custom=ubuntu-vulkan:16.04
Xbuildworker:Custom=linux-4
Xtest_bigdata:Custom=1
Xtests_filter:Custom=*

buildworker:Custom=linux-4,linux-6
build_image:Custom=ubuntu-cuda:18.04

allow_multiple_commits=1

@asmorkalov asmorkalov requested a review from rogday March 1, 2022 05:06
Copy link
Copy Markdown
Member

@rogday rogday left a comment

Choose a reason for hiding this comment

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

Thank you for contribution!

// If layer said that it could work in-place and layers after it
// no longer use input blob, we'll set output = input.
bool supportInPlace;
LayerShapes() {supportInPlace = false;}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Initializer list?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Code is moved without changes. No plan to refactor it now.

Comment on lines +16 to +17
String model = _model;
String config = _config;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If we make a copy of input parameters either way, it makes more sense to pass them by value instead(const String& model -> String model)

else if (framework == "torch")
CV_Error(Error::StsNotImplemented, "Reading Torch models from buffers");
else if (framework == "dldt")
return readNetFromModelOptimizer(bufferConfig, bufferModel);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can also add onnx buffer support, which is useful for ONNXBackendAPI implementation.

else if (framework == "onnx")
        return readNetFromONNX(bufferModel); 

{
if (crop)
{
float resizeFactor = std::max(size.width / (float)imgSize.width,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I propose to use this opportunity to replace such instances of c-style casts witho static_casts everywhere. You can grep for (float), (int), etc.

CV__DNN_INLINE_NS_BEGIN


Layer::Layer() { preferableTarget = DNN_TARGET_CPU; }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Initializer list?

Comment on lines +27 to +30
bool equal(const LayerPin& r) const
{
return (lid == r.lid && oid == r.oid);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We have an operator ==, std::tie?


bool operator<(const LayerPin& r) const
{
return lid < r.lid || (lid == r.lid && oid < r.oid);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

std::tie?


static Ptr<BackendWrapper> create(Mat& m)
{
return Ptr<BackendWrapper>(new OpenCLBackendWrapper(m));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we grep for new and replace it with makePtr?

CV_TRACE_FUNCTION();
if (preferableBackend == DNN_BACKEND_OPENCV)
{
CV_Assert(preferableTarget == DNN_TARGET_CPU || IS_DNN_OPENCL_TARGET(preferableTarget));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We could add initOpenCVBackend() as well. This would help get rid of copies in situations where blob preparation is destructive, so we have to keep a copy for some other backend to act on.

Comment on lines +18 to +19
const int qmin = -128; // INT8_MIN
const int qmax = 127; // INT8_MAX
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we can replace this magic constants

Copy link
Copy Markdown
Member

@rogday rogday left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@opencv-pushbot opencv-pushbot merged commit 685797f into opencv:4.x Mar 17, 2022
@alalek alalek mentioned this pull request Mar 17, 2022
6 tasks
@opencv-pushbot opencv-pushbot mentioned this pull request Apr 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants