From d42fbed02daf63d29be3b6383f017a6f4e7c7c9e Mon Sep 17 00:00:00 2001 From: Yu Yang Date: Sun, 20 Nov 2016 22:12:32 +0800 Subject: [PATCH] Fix several cpp issues * Different Type compare. * ostream << should pass a const object. * remove always true checks. --- .../gserver/evaluators/CTCErrorEvaluator.cpp | 2 +- paddle/gserver/evaluators/ChunkEvaluator.cpp | 2 +- paddle/gserver/evaluators/Evaluator.cpp | 6 +++--- paddle/gserver/evaluators/Evaluator.h | 16 +++++++-------- .../gserver/gradientmachines/MultiNetwork.cpp | 2 +- .../gradientmachines/NeuralNetwork.cpp | 2 +- paddle/math/BaseMatrix.cu | 20 +++++++++---------- paddle/math/Vector.cpp | 6 +++--- paddle/pserver/ParameterServer2.cpp | 2 -- paddle/utils/BarrierStat.cpp | 10 +++++----- paddle/utils/BarrierStat.h | 11 +++++----- paddle/utils/CompilerMacros.h | 17 ++++++++++++++++ paddle/utils/Logging.cpp | 4 ++-- paddle/utils/Logging.h | 3 ++- 14 files changed, 60 insertions(+), 43 deletions(-) create mode 100644 paddle/utils/CompilerMacros.h diff --git a/paddle/gserver/evaluators/CTCErrorEvaluator.cpp b/paddle/gserver/evaluators/CTCErrorEvaluator.cpp index e397c71c87..c2625bce9a 100644 --- a/paddle/gserver/evaluators/CTCErrorEvaluator.cpp +++ b/paddle/gserver/evaluators/CTCErrorEvaluator.cpp @@ -240,7 +240,7 @@ public: seqClassficationError_ = 0; } - virtual void printStats(std::ostream& os) { + virtual void printStats(std::ostream& os) const { os << config_.name() << "=" << (numSequences_ ? totalScore_ / numSequences_ : 0); os << " deletions error" diff --git a/paddle/gserver/evaluators/ChunkEvaluator.cpp b/paddle/gserver/evaluators/ChunkEvaluator.cpp index 22579891f3..6f5d2b47c3 100644 --- a/paddle/gserver/evaluators/ChunkEvaluator.cpp +++ b/paddle/gserver/evaluators/ChunkEvaluator.cpp @@ -114,7 +114,7 @@ public: numCorrect_ = 0; } - virtual void printStats(std::ostream& os) { + virtual void printStats(std::ostream& os) const { double precision = (double)numCorrect_ / numOutputSegments_; double recall = (double)numCorrect_ / numLabelSegments_; double f1 = diff --git a/paddle/gserver/evaluators/Evaluator.cpp b/paddle/gserver/evaluators/Evaluator.cpp index 7bdcdaae53..d43dceea74 100644 --- a/paddle/gserver/evaluators/Evaluator.cpp +++ b/paddle/gserver/evaluators/Evaluator.cpp @@ -315,7 +315,7 @@ public: return 0; } - virtual void printStats(std::ostream& os) { + virtual void printStats(std::ostream& os) const { CHECK(colIdx_ + (int32_t)colNum_ >= 0 && colIdx_ - (int32_t)colNum_ < 0) << "column index [" << colIdx_ << "] out of range [-" << colNum_ << ", " << colNum_ << ")"; @@ -421,7 +421,7 @@ void AucEvaluator::distributeEval(ParameterClient2* client) { client->reduce(statNeg_, statNeg_, kBinNum_ + 1, FLAGS_trainer_id, 0); } -double AucEvaluator::calcAuc() { +double AucEvaluator::calcAuc() const { double totPos = 0.0; double totNeg = 0.0; double totPosPrev = 0.0; @@ -584,7 +584,7 @@ real PrecisionRecallEvaluator::evalImp(std::vector& arguments) { return 0; } -void PrecisionRecallEvaluator::printStats(std::ostream& os) { +void PrecisionRecallEvaluator::printStats(std::ostream& os) const { int label = config_.positive_label(); if (label != -1) { CHECK(label >= 0 && label < (int)statsInfo_.size()) diff --git a/paddle/gserver/evaluators/Evaluator.h b/paddle/gserver/evaluators/Evaluator.h index b79a539384..e9957a5ce2 100644 --- a/paddle/gserver/evaluators/Evaluator.h +++ b/paddle/gserver/evaluators/Evaluator.h @@ -99,19 +99,19 @@ public: * @brief print the statistics of evaluate result * @note finish() should be called before printStats */ - virtual void printStats(std::ostream& os) { + virtual void printStats(std::ostream& os) const { os << config_.name() << "=" << (numSamples_ ? totalScore_ / numSamples_ : 0); } friend std::ostream& operator<<(std::ostream& os, - Evaluator& evaluator) { + const Evaluator& evaluator) { evaluator.printStats(os); return os; } friend std::ostream&& operator<<(std::ostream&& os, // NOLINT - Evaluator& evaluator) { + const Evaluator& evaluator) { evaluator.printStats(os); return std::move(os); } @@ -135,7 +135,7 @@ public: return -1; } virtual void finish() {} - virtual void printStats(std::ostream&) {} + virtual void printStats(std::ostream&) const {} }; /** * @brief evaluate AUC using colIdx-th column as prediction. @@ -165,7 +165,7 @@ public: virtual real evalImp(std::vector& arguments); - virtual void printStats(std::ostream& os) { + virtual void printStats(std::ostream& os) const { os << config_.name() << "=" << calcAuc(); } @@ -189,7 +189,7 @@ private: return (X1 > X2 ? (X1 - X2) : (X2 - X1)) * (Y1 + Y2) / 2.0; } - double calcAuc(); + double calcAuc() const; }; /** @@ -244,7 +244,7 @@ public: virtual real evalImp(std::vector& arguments); - virtual void printStats(std::ostream& os); + virtual void printStats(std::ostream& os) const; virtual void distributeEval(ParameterClient2* client); @@ -339,7 +339,7 @@ public: virtual void finish() { calc(predictArray_); } - virtual void printStats(std::ostream& os) { + virtual void printStats(std::ostream& os) const { os << " pos/neg" << "=" << pairArray_[0] / ((pairArray_[1] <= 0) ? 1.0 : pairArray_[1]); } diff --git a/paddle/gserver/gradientmachines/MultiNetwork.cpp b/paddle/gserver/gradientmachines/MultiNetwork.cpp index d30ca6f28e..b85d2e0c99 100644 --- a/paddle/gserver/gradientmachines/MultiNetwork.cpp +++ b/paddle/gserver/gradientmachines/MultiNetwork.cpp @@ -154,7 +154,7 @@ public: return -1; } - virtual void printStats(std::ostream& os) { + virtual void printStats(std::ostream& os) const { for (auto& evaluator : evaluators_) { evaluator->printStats(os); os << ' '; diff --git a/paddle/gserver/gradientmachines/NeuralNetwork.cpp b/paddle/gserver/gradientmachines/NeuralNetwork.cpp index 3127b4dd9a..c77b00eb06 100644 --- a/paddle/gserver/gradientmachines/NeuralNetwork.cpp +++ b/paddle/gserver/gradientmachines/NeuralNetwork.cpp @@ -325,7 +325,7 @@ public: (void)arguments; return -1; } - virtual void printStats(std::ostream& os) { + virtual void printStats(std::ostream& os) const { for (auto& evaluator : evaluators_) { evaluator->printStats(os); os << ' '; diff --git a/paddle/math/BaseMatrix.cu b/paddle/math/BaseMatrix.cu index 54448bdb5a..2afb216db5 100644 --- a/paddle/math/BaseMatrix.cu +++ b/paddle/math/BaseMatrix.cu @@ -1449,8 +1449,8 @@ template<> template int BaseMatrixT::applyRow(Agg agg, BaseMatrixT& b) { MatrixOffset offset(0, 0, 0, 0, 0, 0); - int numRows = b.height_; - int numCols = b.width_; + auto numRows = b.height_; + auto numCols = b.width_; CHECK_EQ(height_, numRows); CHECK_EQ(width_, 1UL); aggregate(agg, base::unary::identity(), base::binary::second(), b, numRows, @@ -1463,8 +1463,8 @@ template<> template int BaseMatrixT::applyRow(Agg agg, Saver sv, BaseMatrixT& b) { MatrixOffset offset(0, 0, 0, 0, 0, 0); - int numRows = b.height_; - int numCols = b.width_; + auto numRows = b.height_; + auto numCols = b.width_; CHECK_EQ(height_, numRows); CHECK_EQ(width_, 1UL); aggregate(agg, base::unary::identity(), sv, b, numRows, numCols, offset, @@ -1493,8 +1493,8 @@ template int BaseMatrixT::applyRow(Agg agg, Op op, Saver sv, BaseMatrixT& b, BaseMatrixT& c) { MatrixOffset offset(0, 0, 0, 0, 0, 0); - int numRows = b.height_; - int numCols = b.width_; + auto numRows = b.height_; + auto numCols = b.width_; CHECK_EQ(height_, numRows); CHECK_EQ(width_, 1UL); CHECK_EQ(c.height_, numRows); @@ -1524,8 +1524,8 @@ template<> template int BaseMatrixT::applyCol(Agg agg, BaseMatrixT& b) { MatrixOffset offset(0, 0, 0, 0, 0, 0); - int numRows = b.height_; - int numCols = b.width_; + auto numRows = b.height_; + auto numCols = b.width_; CHECK_EQ(width_, numCols); CHECK_EQ(height_, 1UL); aggregate(agg, base::unary::identity(), base::binary::second(), b, numRows, @@ -1538,8 +1538,8 @@ template<> template int BaseMatrixT::applyCol(Agg agg, Saver sv, BaseMatrixT& b) { MatrixOffset offset(0, 0, 0, 0, 0, 0); - int numRows = b.height_; - int numCols = b.width_; + auto numRows = b.height_; + auto numCols = b.width_; CHECK_EQ(width_, numCols); CHECK_EQ(height_, 1UL); aggregate(agg, base::unary::identity(), sv, b, numRows, numCols, offset, diff --git a/paddle/math/Vector.cpp b/paddle/math/Vector.cpp index 23c9caccea..9ef7f2b4b5 100644 --- a/paddle/math/Vector.cpp +++ b/paddle/math/Vector.cpp @@ -82,8 +82,8 @@ MatrixPtr VectorT::toOneHotSparseMatrix(size_t idRange, bool useGpu) { template <> MatrixPtr VectorT::toOneHotSparseMatrix(size_t idRange, bool useGpu) { - int height = getSize(); - int width = idRange; + auto height = getSize(); + auto width = idRange; MatrixPtr mat = Matrix::createSparseMatrix( height, idRange, height, NO_VALUE, SPARSE_CSR, false, useGpu); @@ -91,7 +91,7 @@ MatrixPtr VectorT::toOneHotSparseMatrix(size_t idRange, bool useGpu) { cpuIds.copyFrom(*this); int *idData = cpuIds.getData(); - for (int i = 0; i < height; i ++) { + for (decltype(height) i = 0; i < height; i ++) { const unsigned int id = idData[i]; CHECK_LT(id, width); mat->setRow(i, 1, &id, nullptr); diff --git a/paddle/pserver/ParameterServer2.cpp b/paddle/pserver/ParameterServer2.cpp index c8f37d0bf4..960fca2853 100644 --- a/paddle/pserver/ParameterServer2.cpp +++ b/paddle/pserver/ParameterServer2.cpp @@ -1469,7 +1469,6 @@ void ParameterServer2::waitPassFinish(const WaitPassFinishRequest& request, void ParameterServer2::synchronize(const SynchronizeRequest& request, ProtoResponseCallback callback) { - CHECK_LT(request.sync_object_id(), SyncObject_ARRAYSIZE); synchronizeBarriers_[request.sync_object_id()]->wait(); dataSize_ = 0; callback(SynchronizeResponse()); @@ -1477,7 +1476,6 @@ void ParameterServer2::synchronize(const SynchronizeRequest& request, void ParameterServer2::asyncFinishPass(const SynchronizeRequest& request, ProtoResponseCallback callback) { - CHECK_LT(request.sync_object_id(), SyncObject_ARRAYSIZE); synchronizeBarriers_[request.sync_object_id()]->wait(); callback(SynchronizeResponse()); diff --git a/paddle/utils/BarrierStat.cpp b/paddle/utils/BarrierStat.cpp index cbc738a839..f083ef3982 100644 --- a/paddle/utils/BarrierStat.cpp +++ b/paddle/utils/BarrierStat.cpp @@ -29,10 +29,10 @@ P_DEFINE_bool(log_barrier_show_log, false, // for performance tuning insight namespace paddle { -std::ostream &operator<<(std::ostream &output, BarrierStatBase &stat) { +std::ostream &operator<<(std::ostream &output, + const BarrierStatBase &stat) { if (FLAGS_log_barrier_abstract) { - std::lock_guard guard( - const_cast(stat).lock_); + std::lock_guard guard(stat.lock_); stat.showAbstract(output); } return output; @@ -136,7 +136,7 @@ void BarrierEndStat::reset(bool clearRawData) { totAbstract_.minDelta = UINT64_MAX; } -void BarrierEndStat::showAbstract(std::ostream &output) { +void BarrierEndStat::showAbstract(std::ostream &output) const { // do not support the case "<=2 pserver" if (numConnThreads_ <= 2 || !totSamples_) { return; @@ -272,7 +272,7 @@ void BarrierDeltaStat::reset(bool clearRawData) { totAbstract_.minDelta = UINT64_MAX; } -void BarrierDeltaStat::showAbstract(std::ostream &output) { +void BarrierDeltaStat::showAbstract(std::ostream &output) const { // do not support the case "<=2 pserver" if (numConnThreads_ <= 2 || !totSamples_) { return; diff --git a/paddle/utils/BarrierStat.h b/paddle/utils/BarrierStat.h index 22d6cc9bce..add1093758 100644 --- a/paddle/utils/BarrierStat.h +++ b/paddle/utils/BarrierStat.h @@ -218,11 +218,12 @@ public: } protected: - virtual void showAbstract(std::ostream &output) {} - friend std::ostream &operator<<(std::ostream &output, BarrierStatBase &stat); + virtual void showAbstract(std::ostream &output) const {} + friend std::ostream &operator<<(std::ostream &output, + const BarrierStatBase &stat); protected: - std::mutex lock_; + mutable std::mutex lock_; std::mutex abstractLock_; // see note on updaterStat // each freqency for each barrier trainer std::vector abstract_; @@ -262,7 +263,7 @@ protected: * log_barrier_abstract, log_barrier_lowest_nodes, log_barrier_threshold * control details. */ - virtual void showAbstract(std::ostream &output); + virtual void showAbstract(std::ostream &output) const; private: std::unique_ptr timeVector_; @@ -286,7 +287,7 @@ public: virtual bool checkPassBarrier() { return timeVector_->empty(); } protected: - virtual void showAbstract(std::ostream &outPut); + virtual void showAbstract(std::ostream &outPut) const; private: // store delta time in uint64_t, eg BP time of all trainers diff --git a/paddle/utils/CompilerMacros.h b/paddle/utils/CompilerMacros.h new file mode 100644 index 0000000000..4236d750c4 --- /dev/null +++ b/paddle/utils/CompilerMacros.h @@ -0,0 +1,17 @@ +/* Copyright (c) 2016 Baidu, Inc. All Rights Reserve. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. */ + +#pragma once + +#define ATTR_NORETURN __attribute__((noreturn)) diff --git a/paddle/utils/Logging.cpp b/paddle/utils/Logging.cpp index a0644940b5..9a6b1f2d83 100644 --- a/paddle/utils/Logging.cpp +++ b/paddle/utils/Logging.cpp @@ -134,7 +134,7 @@ static void initializeLogFds(char* argv0) { gLogInited = true; } -static void (*gFailureFunctionPtr)() __attribute__((noreturn)) = abort; +static void (*gFailureFunctionPtr)() ATTR_NORETURN = abort; LogMessage::LogMessage(const char* fname, int line, int severity) : fname_(fname), line_(line), severity_(severity) {} @@ -171,7 +171,7 @@ void setMinLogLevel(int level) { paddle::internal::gMinLogLevel = level; } -void installFailureFunction(void (*callback)()) { +void installFailureFunction(void (*callback)() ATTR_NORETURN) { paddle::internal::gFailureFunctionPtr = callback; } diff --git a/paddle/utils/Logging.h b/paddle/utils/Logging.h index 7fdfa3240c..46b6a7feeb 100644 --- a/paddle/utils/Logging.h +++ b/paddle/utils/Logging.h @@ -23,6 +23,7 @@ limitations under the License. */ #include #ifndef PADDLE_USE_GLOG +#include "CompilerMacros.h" //! TODO(yuyang18): Move this utility macro into some global header. #define PP_CAT(a, b) PP_CAT_I(a, b) @@ -168,7 +169,7 @@ void setMinLogLevel(int level); * @brief Install Log(Fatal) failure function. Default is abort(); * @param callback: The failure function. */ -void installFailureFunction(void (*callback)()); +void installFailureFunction(void (*callback)() ATTR_NORETURN); /** * @brief installFailureWriter -- GitLab