Please, add an assertion for unknown layer types to prevent unexpected errors. In example, I can't read any model now because an every layer_type ends with ] character (convolutional], maxpool]). (Ubuntu OS).

It works now but Reproducibility_TinyYoloVoc and Reproducibility_YoloVoc tests are failed for me. Do them passed locally?

We defined methods like createCaffeImporter as deprecated. Please keep only readNetFromDarknet.

It isn't necessary: just use imread with default argument (http://docs.opencv.org/master/d4/da8/group__imgcodecs.html#ga288b8b3da0892bd651fce07b3bbd3a56).

@AlexeyAB, thanks, but I meant that cv::imread can read images with alpha into 24bit, http://docs.opencv.org/master/d4/da8/group__imgcodecs.html#gga61d9b0126a3e57d9277ac48327799c80af660544735200cbe942eea09232eb822.

@dkurt I fixed it.
Initially I did it as in ssd_object_detection.cpp and I thought that this has some hidden meaning :)

Please, use blobFromImage's arguments to make preprocessing (http://docs.opencv.org/3.3.0/d6/d0f/group__dnn.html#ga0507466a789702eda8ffcdfa37f4d194).

Is there some difference between Route layer and Concat? getMemoryShapes returns true if layer can work in-place (all element-wise layers).

I don't know why, but it doesn't work for Yolo if getMemoryShapes returns false.
Route-layer simply copies unchanged outputs from several layers: https://github.com/pjreddie/darknet/blob/master/src/route_layer.c#L83

Used copy_cpu() with INCX=1 and INCY=1: https://github.com/pjreddie/darknet/blob/master/src/blas.c#L208

@AlexeyAB, it seems to me problem is in route layer with a single input (that means problem is in current concat layer with #inputs == 1), https://github.com/pjreddie/darknet/blob/master/cfg/yolo-voc.cfg#L208. Is it used like an identity layer, right?

@dkurt Yes, route with a single input (bottom layer) is used as identity layer.

@AlexeyAB, could you add an extra branch during route layer creation: add Concat layer if number of inputs more than 1 or Identity layer otherwise?

@dkurt I added identity layer for the 1 input. But why concat layer can't work with 1 input, and why there is no CV_Assert for this case?

I'm a bit confused about reorg layer. Let the input is:

channel_0  channel_1  channel_2  channel_3
0 1        4 5        8 9        c d
2 3        6 7        a b        e f

and reorgStride = 2. So an output shape is 4x4x1 and values are:

output
1 4 1 5
8 c 9 d
2 6 3 7
a e b f

?

I left unchanged a bit strange original implementation of this layer. It increases the field of view of each final activation.
Reshape: 26 x 26 x 64 -> 13 x 13 x 256

For stride = 2

input
    0, 1, 2, 3,
    4, 5, 6, 7, 
    8, 9, a, b,
    c, d, e, f

output
channel_0  channel_1  channel_2  channel_3
0 2        1 3        4 6        5 7
8 a        9 b        c e        d f

• OpenCV C++ example: http://coliru.stacked-crooked.com/a/eb13942be083fa3d
• Darknet C++ example: http://coliru.stacked-crooked.com/a/225d7fb1f25b286c

• Param reverse usually absent in the cfg-file of model, so reverse = 0 by default https://github.com/pjreddie/darknet/blob/master/src/parser.c#L387

• Then l.reverse = reverse; https://github.com/pjreddie/darknet/blob/master/src/reorg_layer.c#L28

• So the function reorg() is called with forward=0: https://github.com/pjreddie/darknet/blob/8215a8864d4ad07e058acafd75b2c6ff6600b9e8/src/reorg_layer.c#L108

• reorg_cpu()-implementation uses out[in_index] = x[out_index];: https://github.com/pjreddie/darknet/blob/master/src/blas.c#L25

Thanks! Anyway I suggest to replace it as a single layer or think how we can do the same transformations using existing ones (Permute, Reshape). Reshape layer doesn't change the data by definition neither in one of the frameworks.

@dkurt I added reorg as separate layer reorg_layer.cpp

Magic number?

Why we track shapes? Doesn't weights file contain kernels shapes?

Yes, weights file doesn't contain kernels shapes.
Also Darknet tracks layers shapes while parsing a cfg-file:

• parse_network_cfg() https://github.com/pjreddie/darknet/blob/master/src/parser.c#L630
  • parse_convolutional() https://github.com/pjreddie/darknet/blob/master/src/parser.c#L169
    • make_convolutional_layer() https://github.com/pjreddie/darknet/blob/master/src/convolutional_layer.c#L166
      • convolutional_out_height() https://github.com/pjreddie/darknet/blob/master/src/convolutional_layer.c#L66

int convolutional_out_height(convolutional_layer l)
{
    return (l.h + 2*l.pad - l.size) / l.stride + 1;
}

@AlexeyAB, May be we can remove at least width/height sizes tracking? As I can see only current_shape.input_channels is used to read convolutional layer weights.

Version numbers are used only to decide how many bytes to skip for seen value. transpose aren't used at all. Please make all unused variables from NetParameter are local.

@AlexeyAB, it seems to me problem is in route layer with a single input (that means problem is in current concat layer with #inputs == 1), https://github.com/pjreddie/darknet/blob/master/cfg/yolo-voc.cfg#L208. Is it used like an identity layer, right?

Please, setup only actual parameters: "pool", "kernel_size", "pad", "stride".

Ok. Also required maxpool_param.set<cv::String>("pad_mode", "SAME"); for odd sizes of layers.

However only one way of padding strategy is used: manual values or padMode ("SAME", "VALID") from TensorFlow. Please take a look on the "ceil_mode" flag instead: https://github.com/opencv/opencv/blob/master/modules/dnn/src/layers/pooling_layer.cpp#L629.

• Accuracy test passed for Tiny-Yolo if padMode="SAME" with any ceil_mode value.
• Accuracy test can't be passed for Tiny-Yolo for any values of padMode (padMode="VALID" or if padMod isn't set) with any ceil_mode value.

Is there no typo in in<->out indices place?

No, there is no typo, initially I left unchanged a bit strange original implementation of this layer.
But now I have changed this place so that there is no confusion.

It seems to me Reorg layer can't work in-place. getMemoryShapes returns true if layer can do it.

The same as Reorg layer: it should returns false.

It's better to name layers using type prefix every time. Moreover some of layers just named with numbers and it's hard to debug them.

Unused variable channels_num

Unused variable channels_num

Unused structure

Unused function

Please, add an assertion that total(outputs[0]) == total(inputs[0]).

Please make it more clear: iterate over output dimensions and map them to input ones.

Most likely there was made a logical mistake in the original version of Darknet in the reorg layer: https://github.com/pjreddie/darknet/blob/master/src/blas.c#L9

It works as I described if called reorg(input, output, out_w, out_h, _out_c, ...); #9705 (comment)

But in the original Darknet version function is called in this way reorg(input, output, in_w, in_h, in_c, ...);, so the one-to-one correspondence of the input and output parameters is preserved, but very strange permutations occur.

But because original Darknet works with this implementation of reorg and all models trained using it, then we can't fix this logical mistake.

Why the author has not found and corrected this error? I think:

• Perhaps this logical error does not spoil the detection accuracy, so it was not detected.
• Theoretically, we can assume that this error even increased the accuracy - so the author found it and left it.

iterate over output dimensions and map them to input ones.

So I can implement it in a such way,
but it works correctly only if (in_w % 2 == 0 && in_h % 2 == 0 && in_c % 4 == 0) : http://coliru.stacked-crooked.com/a/b962d20938362d4f

void reorg_my(const float*const srcData,  float *const dstData, int width, int height, int channels, int reorgStride)
{
	int outChannels = channels * reorgStride * reorgStride;
	int outHeight = height / reorgStride;
	int outWidth = width / reorgStride;

	for (int y = 0; y < outHeight; ++y) {
		for (int x = 0; x < outWidth; ++x) {
			for (int c = 0; c < outChannels; ++c) {
				int out_index = x + outWidth*(y + outHeight*c);

				int step = c / channels;
				int x_offset = step % reorgStride;
				int y_offset = reorgStride * ((step / reorgStride) % reorgStride);

				int in_x = x * reorgStride + x_offset;
				
				int out_seq_y = y + c*outHeight;
				int in_intermediate_y = out_seq_y*2 - out_seq_y%2;
				in_intermediate_y = in_intermediate_y % (channels*height);
				int in_c = in_intermediate_y / height;
				int in_y = in_intermediate_y % height + y_offset;
						
				int in_index = in_x + width*(in_y + height*in_c);
				dstData[out_index] = srcData[in_index];
			}
		}
	}
}

Most likely there was made a logical mistake in the original version of Darknet in the reorg layer: https://github.com/pjreddie/darknet/blob/master/src/blas.c#L9

It works as I described if called reorg(input, output, out_w, out_h, _out_c, ...); #9705 (comment)

But in the original Darknet version function is called in this way reorg(input, output, in_w, in_h, in_c, ...);, so the one-to-one correspondence of the input and output parameters is preserved, but very strange permutations occur.

But because original Darknet works with this implementation of reorg and all models trained using it, then we can't fix this logical mistake.

Why the author has not found and corrected this error? I think:

* Perhaps this logical error does not spoil the detection accuracy, so it was not detected.

* Theoretically, we can assume that this error even increased the accuracy - so the author found it and left it.

iterate over output dimensions and map them to input ones.

So I can implement it in a such way,
but it works correctly only if (in_w % 2 == 0 && in_h % 2 == 0 && in_c % 4 == 0) : http://coliru.stacked-crooked.com/a/b962d20938362d4f

void reorg_my(const float*const srcData,  float *const dstData, int width, int height, int channels, int reorgStride)
{
	int outChannels = channels * reorgStride * reorgStride;
	int outHeight = height / reorgStride;
	int outWidth = width / reorgStride;

	for (int y = 0; y < outHeight; ++y) {
		for (int x = 0; x < outWidth; ++x) {
			for (int c = 0; c < outChannels; ++c) {
				int out_index = x + outWidth*(y + outHeight*c);

				int step = c / channels;
				int x_offset = step % reorgStride;
				int y_offset = reorgStride * ((step / reorgStride) % reorgStride);

				int in_x = x * reorgStride + x_offset;
				
				int out_seq_y = y + c*outHeight;
				int in_intermediate_y = out_seq_y*2 - out_seq_y%2;
				in_intermediate_y = in_intermediate_y % (channels*height);
				int in_c = in_intermediate_y / height;
				int in_y = in_intermediate_y % height + y_offset;
						
				int in_index = in_x + width*(in_y + height*in_c);
				dstData[out_index] = srcData[in_index];
			}
		}
	}
}

Is there a GPU version of reorg？

float const*const is a bit confusing (there are 4 places with it).

May I ask you to use named constant variables or place a comments because it's hard to understand the magic numbers? Especially it's about samples and tests.

I removed const*const, also added named constant variables and described the format of network output that compared to the reference in the tests.

But why is const*const confusing, is this contrary to the code style conventions that is accepted in OpenCV?
1st const forbids modification of values pointed to by this pointer, 2nd const forbids modification of this pointer.

Please try to use cv::format("conv_%d", layer_id) instead toString here and in other places.

I hope we can emit this structure. Layers are connected sequentially or using explicit numeric offsets starting from the newly added layer. So I think it's possible to use single vector of layers during network building. May I ask you to try it?

Do you mean that I should to try use cv::dnn::experimental_dnn_v1::LayerParams instead of darknet::LayerParameter?

Yeah, I think we can just parse text and binary files simultaneously: for an every entry in config we create a new one LayerParams and fill it depends on layer type. If specific layer has weights - read them from opened binary file. Then add a layer to final network (addLayerToPrev or addLayer with multiple connections based on id of the new layer and offsets i.e. -1, -4 of route).

@AlexeyAB, on the other hand, let's keep it as is now. I'll just install darknet and compare it with PR and we can merge it.