Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DetectionOutputLayer and MultiBoxLossLayer. #2497

Merged
merged 5 commits into from
Jul 4, 2017

Conversation

pkuyym
Copy link
Contributor

@pkuyym pkuyym commented Jun 18, 2017

resolves #2496

@pkuyym pkuyym requested a review from qingqing01 June 18, 2017 09:18
locBuffer_ = locCpuBuffer_;
confBuffer_ = confCpuBuffer_;
priorValue = priorCpuValue_;
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

这里觉得,上面appendWithPermute直接操作locTmpBuffer_ confTmpBuffer_ ,在else分支里再给locBuffer_ confBuffer_ 赋值逻辑更清晰些:

locBuffer_ = locTmpBuffer_;
confBuffer_ = confTmpBuffer_;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}
confBuffer_->softmax(*confBuffer_);

size_t numPriors = priorValue->getElementCnt() / 8;
Copy link
Contributor

Choose a reason for hiding this comment

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

同上个PR,8、4、7这样的常量需要重新定义,避免直接在代码里用数字,不结合上下文,不能直接看懂。 不过后续重构用Tensor更好实现些~


using std::vector;
using std::map;
using std::pair;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not use aliases for std::vector, std::map and std::pair in the header. https://google.github.io/styleguide/cppguide.html#Aliases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

* The detection output layer for a SSD detection task. This layer apply the
* Non-maximum suppression to the all predicted bounding box and keep the
* Top-K bounding boxes.
* - Input: This layer need three input layers: This first input layer
Copy link
Contributor

Choose a reason for hiding this comment

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

need -> needs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

confProb_->getData()[count * numClasses_ + j] =
(confBuffer_->getData() + confOffset)[j];
confPredData.push_back((confBuffer_->getData() + confOffset)[j]);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

for循环里的拷贝可以使用std::copy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@config_layer('multibox_loss')
class MultiBoxLossLayer(LayerBase):
def __init__(self, name, inputs, input_num, num_classes, overlap_threshold,
neg_pos_ratio, neg_overlap, background_id):
Copy link
Contributor

Choose a reason for hiding this comment

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

background_id): -> background_id, **xargs):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

class DetectionOutputLayer(LayerBase):
def __init__(self, name, inputs, size, input_num, num_classes,
nms_threshold, nms_top_k, keep_top_k, confidence_threshold,
background_id):
Copy link
Contributor

Choose a reason for hiding this comment

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

background_id): -> background_id, **xargs):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

assert isinstance(input_loc, collections.Sequence) # list or tuple
for each in input_loc:
assert isinstance(each, LayerOutput)
input_loc_num += 1
Copy link
Contributor

Choose a reason for hiding this comment

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

可以去掉前面input_loc_num=0,改成:

for each in input_loc:
    assert isinstance(each, LayerOutput)
input_loc_num = len(input_loc)

input_conf_num同样~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

这块没有Done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

assert isinstance(input_conf, collections.Sequence) # list or tuple
for each in input_conf:
assert isinstance(each, LayerOutput)
input_conf_num += 1
Copy link
Contributor

Choose a reason for hiding this comment

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

同上~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

这块也没有Done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -115,6 +115,8 @@
'print_layer',
'priorbox_layer',
'cross_channel_norm_layer',
'multibox_loss_layer',
'detection_output_layer',
Copy link
Contributor

Choose a reason for hiding this comment

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

增加到文档里,同时增加单测~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All unit tests have been added. Which document?

Copy link
Contributor

Choose a reason for hiding this comment

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

doc/api/v2/config/layer.rst

real loss = locLoss_ + confLoss_;
MatrixPtr outV = getOutputValue();
std::vector<real> tmp(batchSize, loss);
outV->copyFrom(&tmp[0], batchSize);
Copy link
Contributor

Choose a reason for hiding this comment

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

每个样本的loss都一样?如果是,直接赋值给outV就可以,不用中间构造:

std::vector<real> tmp(batchSize, loss);

看下 BaseMatrix::assign是否可用。

size_t height = getInput(*getLocInputLayer(n)).getFrameHeight();
if (!height) height = layerConf.height();
size_t width = getInput(*getLocInputLayer(n)).getFrameWidth();
if (!width) width = layerConf.width();
Copy link
Contributor

Choose a reason for hiding this comment

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

layerConf.height()layerConf.width()为了单测才加的,仅仅在这里注释下。

size_t height = getInput(*getLocInputLayer(n)).getFrameHeight();
if (!height) height = layerConf.height();
size_t width = getInput(*getLocInputLayer(n)).getFrameWidth();
if (!width) width = layerConf.width();
Copy link
Contributor

Choose a reason for hiding this comment

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

size_t width = getInput(*getLocInputLayer(n)).getFrameWidth();

这句理解,每个LocInputLayer的height/width可以不一样,实际也不一样吧? 但为了单测加的:

if (!width) width = layerConf.width();

确实每个LocInputLayer的height/width都一样。

* The loss is composed by the location loss and the confidence loss.
* The location loss is a smooth L1 loss and the confidence loss is
* a softmax loss.
* - Input: This layer need four input layers: This first input layer
Copy link
Contributor

Choose a reason for hiding this comment

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

need -> needs
This first input -> The first input

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

namespace paddle {

/**
* The detection output layer for a SSD detection task. This layer apply the
Copy link
Contributor

Choose a reason for hiding this comment

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

apply -> applies

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

:param name: The Layer Name.
:type name: basestring
:param input_loc: The input predict location.
:type input_loc: LayerOutput
Copy link
Contributor

Choose a reason for hiding this comment

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

LayerOutput | List of LayerOutput.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

:param input_loc: The input predict location.
:type input_loc: LayerOutput
:param input_conf: The input priorbox confidence.
:type input_conf: LayerOutput
Copy link
Contributor

Choose a reason for hiding this comment

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

LayerOutput | List of LayerOutput.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

assert isinstance(input_loc, collections.Sequence) # list or tuple
for each in input_loc:
assert isinstance(each, LayerOutput)
input_loc_num += 1
Copy link
Contributor

Choose a reason for hiding this comment

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

这块没有Done.

assert isinstance(input_conf, collections.Sequence) # list or tuple
for each in input_conf:
assert isinstance(each, LayerOutput)
input_conf_num += 1
Copy link
Contributor

Choose a reason for hiding this comment

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

这块也没有Done.

@@ -0,0 +1,25 @@
from paddle.trainer_config_helpers import *
Copy link
Contributor

Choose a reason for hiding this comment

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

需要运行下单测,将protostr文件add进来。上同。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@pkuyym pkuyym merged commit 312ce8b into PaddlePaddle:develop Jul 4, 2017
@pkuyym pkuyym deleted the ssd_outloss branch July 4, 2017 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add DetectionOutputLayer and MultiBoxLossLayer for SSD
2 participants