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 SpatialPyramidPoolLayer c++ support #300

Merged
merged 15 commits into from
Nov 11, 2016

Conversation

QiJune
Copy link
Member

@QiJune QiJune commented Nov 1, 2016

1 add SpatialPyamidPoolLayer c++ support #257
2 use clang-format command to format matrix.cpp file
3 add PoolProjection as a data member in projectionLayer to reuse code

testSppLayer("avg", 5, false, useGpu);
testSppLayer("max", 1, false, useGpu);
testSppLayer("max", 3, false, useGpu);
testSppLayer("avg", 5, false, useGpu);
Copy link
Contributor

Choose a reason for hiding this comment

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

905行,应该是测max吧,可以写成

for (auto pool_type : {"avg", "max"})
   for (auto num : {1, 3, 5}) 
       testSppLayer(pool_type, num, false, useGpu); 

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this one looks good

if (!sMatPtr) {
sumCols(a, scale);
} else {
real* data = getData();
hl_sparse_matrix_s A_d = sMatPtr->sMatrix_.get();
hl_sparse_matrix_column_sum(data, A_d, sMatPtr->getHeight(),
width_, scale);
hl_sparse_matrix_column_sum(data, A_d, sMatPtr->getHeight(), width_, scale);
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
Member Author

Choose a reason for hiding this comment

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

I used clang-format command to format Matrix.cpp, so some code is formatted

@@ -194,6 +202,9 @@ message ProjectionConfig {
optional ConvConfig conv_conf = 8;
optional int32 num_filters = 9;

// For pool
optional PoolConfig pool_conf = 10;

Copy link
Contributor

Choose a reason for hiding this comment

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

直接在message最后添加,不要用10

if (!outV.isContiguous()) {
otData = outV.getData() + n * outV.getStride();
otGrad = outGrad.getData() + n * outGrad.getStride();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

outV.getData(), outGrad.getData(), getStride()等都可以用一个变量先存下来,多处同样修改。getStride, outV.getStride, outGrad.getStride三者相同,可以check一下。

Copy link
Contributor

@qingqing01 qingqing01 left a comment

Choose a reason for hiding this comment

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

另外,config_parse.py和layer.py必须添加layer接口。

@@ -91,6 +91,7 @@ extern void hl_expand_feature2col(
* @param[in] paddingH padding height.
* @param[in] paddingW padding width.
* @param[out] tgtData output data.
* @param[in] tgtStride output data stride.
Copy link
Contributor

Choose a reason for hiding this comment

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

注释改成,样本之间的stride, 这样更容易理解些。

projOutput_[i].value = output_.value->subColMatrix(startCol, endCol);
projOutput_[i].grad = output_.grad->subColMatrix(startCol, endCol);
splitInput(projInput_[i], getInput(0).value->getHeight(),
getInput(0).value->getWidth(), useGpu_);
Copy link
Contributor

Choose a reason for hiding this comment

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

这里可以不用splitInput()函数。

Copy link
Member Author

@QiJune QiJune Nov 8, 2016

Choose a reason for hiding this comment

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

1 这里的splitInput是参考caffe的splitLayer,使得每一个PoolProjection的input value是共用一个的,而grad是独立分配的空间
2 不用splitInput函数的意思是,值得是所有的PoolProjection在backward的时候,grad也是共用一个空间吗

Copy link
Contributor

Choose a reason for hiding this comment

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

@QiJune

  • 在Forward的value共用同一个
  for (size_t i = 0; i < pyramidHeight_; i++) {
      poolProjections_[i]->forward(&getInput(0), &projOutput_[i], passType);
   }
  • 在backward也可以共用同一个空间,都累加到input的grad上。
    forward调用之后,每个poolProjections_的const Argument* in_都是相同的,in_.grad也是相同的,每个poolProjections_的backward计算in_.grad的时候是 in_.grad+=等的形式即可。
    不需要每个PoolProjection的grad都有一个自己的空间。

for (size_t i = 0; i < pyramidHeight_; i++) {
if (poolProjections_[i]) {
poolProjections_[i]->backward(callback);
getInput(0).grad->add(*projInput_[i].grad);
Copy link
Contributor

Choose a reason for hiding this comment

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

上面不用splitInput(), 这块也不用add, poolProjections_里面计算处理add操作即可,这块类似mixed_layer。

Copy link
Contributor

Choose a reason for hiding this comment

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

qingqing说的有道理,既然这里全部加到了getInput(0).grad上面,就没必要保存一份独立的projInput_[i].grad了

size_t endCol = 0;
for (size_t i = 0; i < pyramidHeight_; i++) {
poolProjections_.emplace_back(PoolProjection::create(
getConfig(imgSizeW_, imgSizeH_, channels_, i, poolType_),
Copy link
Contributor

Choose a reason for hiding this comment

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

上面从config里面获取imgSizeW_, imgSizeH_,包括这块getConfig,只在init里面调用了一次,这样就要求config里面必须正确填充input的height和width。 在其他conv、pooling、maxout等layer里面,实际在forward里面会根据输入的Argument获取height和width, 然后正确设置输出Argument的Height和Width,而不要求必须在config里面填充,在@骆涛的对CNN支持长方形输入还没有ci之前,config_parse.py里很多解析都不太正确,这样就会出问题。

就是这块逻辑和conv等逻辑不一样,导致必须config里面必须正确填充,这块可以统一下。

Copy link
Member Author

Choose a reason for hiding this comment

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

1 Projection需要从config中读取得到input的height和width,ConProjection和PoolProjection里面都是这样做的,ConvProjection有一个成员函数getConvParams就是解析config的
2 PoolProjectionLayer在计算forward的时候,是根据输入的argument来确定输入的大小
void PoolProjectionLayer::forward(PassType passType) { Layer::forward(passType); const Argument& in = getInput(0); int batchSize = in.value->getHeight(); int size = getSize(); resetOutput(batchSize, size); poolProjection_->forward(&in, &output_, passType); }
3 SppLayer中的output大小的计算是有固定公式的,只与金字塔的高度有关,这个在config_parse.py中我对sppLayer进行了解析,填充output size就可以了,还没有上传这个

Copy link
Contributor

Choose a reason for hiding this comment

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

青青说的应该是像blockExpandLayer的getBlockNum函数中:

  if (imgSizeH_ == 0) {
    imgSizeH_ = blockConf.img_size_y();
  }
  if (imgSizeW_ == 0) {
    imgSizeW_ = blockConf.img_size_x();
  }

用这种方式来获取H和W,这块目前maxoutLayer,@gangliao 的上采样都是这样写的。可以先这样写,之后再统一。不然单测过不了。

Copy link
Member Author

Choose a reason for hiding this comment

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

PoolProjection需要加入getSize()这样的方法吗?
1 Projection的成员变量in_, out_本来就是const修饰
2 拿PoolProjection与PoolProjectionLayer来说,在PoolProjectionLayer里面已经定义了getSize的方法,poolProjection_是它的一个成员变量,在forward的时候,调用的是poolProjection_->forward. 这样就会有两次getSized的操作,外面一次,里面一次
3 对于任意的Projection操作,如果在Layer中使用Projection,比如concatenateLayer,spatialPyramidLayer这样的,最后的output的输出frameHeight 与frameWidth应该由外层的forward指定,里面的projection的设置似乎就没有关系了啊


int outputSize(int imageSize, int windowSize, int padding, int stride) {
return (imageSize - windowSize + 2 * padding) / stride + 1;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

可以调用mathUtils.h里的outputSize函数,不用再写一遍了。这三行可以直接去掉。

int sizeW = std::ceil(imgSizeW / static_cast<double>(numBins));
int remainderW = sizeW * numBins - imgSizeW;
int paddingW = (remainderW + 1) / 2;
int outSizeW = outputSize(imgSizeW, sizeW, paddingW, sizeW);
Copy link
Contributor

Choose a reason for hiding this comment

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

remainderH和remainderW可以不需要,直接放在padding里算


data = data_layer(name='data', size=3200)


Copy link
Contributor

Choose a reason for hiding this comment

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

line10是多余的空行,请删掉

#include "paddle/utils/Logging.h"

namespace paddle {

Copy link
Contributor

Choose a reason for hiding this comment

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

请加上SPP的注释

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 62.614% when pulling bdc9d10 on QiJune:feature/sppnet into 0c7ac3d on baidu:develop.

@coveralls
Copy link

coveralls commented Nov 8, 2016

Coverage Status

Coverage increased (+0.2%) to 62.614% when pulling 70e0468 on QiJune:feature/sppnet into 0c7ac3d on baidu:develop.


return outputY_ * outputX_ * channels_;
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

getSize的实现可以放到cpp文件中,不要放在.h中,69行用getOutput().setFrameHeight(outputH_)比较直观,70行同

Copy link
Member Author

Choose a reason for hiding this comment

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

const_cast<Argument*>(out_)->setFrameHeight(outputY_); const_cast<Argument*>(out_)->setFrameWidth(outputX_);
这样用主要是Projection里面没有getOutput()的接口

}
static PoolProjection* create(const ProjectionConfig& config,
ParameterPtr parameter, bool useGpu);
const std::string& getPoolType() const { return poolType_; }
Copy link
Contributor

@luotao1 luotao1 Nov 9, 2016

Choose a reason for hiding this comment

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

两个函数间要空一行,多处需修改

strideY_ = conf.has_stride_y() ? conf.stride_y() : conf.stride();
confPaddingY_ = conf.has_padding_y() ? conf.padding_y() : conf.padding();
outputY_ = conf.has_output_y() ? conf.output_y() : conf.output_x();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

构造函数的具体实现也能放到cpp中么

Copy link
Member Author

Choose a reason for hiding this comment

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

我改一下

#include "paddle/math/MathUtils.h"

namespace paddle {

Copy link
Contributor

Choose a reason for hiding this comment

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

SPP layer要有注释


settings(
batch_size=100,
learning_rate=1e-5
Copy link
Contributor

Choose a reason for hiding this comment

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

第4行和第5行的缩进不一样


settings(
batch_size=100,
learning_rate=1e-5
Copy link
Contributor

Choose a reason for hiding this comment

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

第4行和第5行的缩进不一样

@luotao1
Copy link
Contributor

luotao1 commented Nov 9, 2016

得加上test_spp_layer.protostr

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 62.653% when pulling eaf3dec on QiJune:feature/sppnet into 0c7ac3d on baidu:develop.

* @brief A layer for spatial pyramid pooling on the input image by taking
* the max, average, etc. within regions, so that the result vector of
* different sized images are of the same size.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

The config file api is spp_layer.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 62.662% when pulling f173341 on QiJune:feature/sppnet into 05204af on baidu:develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 62.638% when pulling 61444d9 on QiJune:feature/sppnet into 8d4c453 on baidu:develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 62.871% when pulling 9dd588b on QiJune:feature/sppnet into 8295eb9 on baidu:develop.

projOutput_[i].value = output_.value->subColMatrix(startCol, endCol);
projOutput_[i].grad = output_.grad->subColMatrix(startCol, endCol);
splitInput(projInput_[i], getInput(0).value->getHeight(),
getInput(0).value->getWidth(), useGpu_);
Copy link
Contributor

Choose a reason for hiding this comment

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

@QiJune

  • 在Forward的value共用同一个
  for (size_t i = 0; i < pyramidHeight_; i++) {
      poolProjections_[i]->forward(&getInput(0), &projOutput_[i], passType);
   }
  • 在backward也可以共用同一个空间,都累加到input的grad上。
    forward调用之后,每个poolProjections_的const Argument* in_都是相同的,in_.grad也是相同的,每个poolProjections_的backward计算in_.grad的时候是 in_.grad+=等的形式即可。
    不需要每个PoolProjection的grad都有一个自己的空间。

@luotao1 luotao1 merged commit ca0bb40 into PaddlePaddle:develop Nov 11, 2016
thisjiang pushed a commit to thisjiang/Paddle that referenced this pull request Oct 28, 2021
wangxicoding pushed a commit to wangxicoding/Paddle that referenced this pull request Dec 9, 2021
* add wordtag linking

* optimize wordtag open

* add termtree_type_csv.csv

* optimize code
qingshui pushed a commit to qingshui/Paddle that referenced this pull request Jun 6, 2023
danleifeng pushed a commit to danleifeng/Paddle that referenced this pull request Sep 13, 2023
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.

5 participants