Skip to content

Commit

Permalink
Fix compiler warnings. (#8022)
Browse files Browse the repository at this point in the history
- Remove/fix unused parameters
- Remove deprecated code in rabit.
- Update dmlc-core.
  • Loading branch information
trivialfis committed Jun 22, 2022
1 parent e44a082 commit 142a208
Show file tree
Hide file tree
Showing 61 changed files with 230 additions and 579 deletions.
4 changes: 3 additions & 1 deletion cmake/Utils.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,9 @@ macro(xgboost_target_properties target)

if (ENABLE_ALL_WARNINGS)
target_compile_options(${target} PUBLIC
$<IF:$<COMPILE_LANGUAGE:CUDA>,-Xcompiler=-Wall -Xcompiler=-Wextra,-Wall -Wextra>
$<IF:$<COMPILE_LANGUAGE:CUDA>,
-Xcompiler=-Wall -Xcompiler=-Wextra -Xcompiler=-Wno-expansion-to-defined,
-Wall -Wextra -Wno-expansion-to-defined>
)
endif(ENABLE_ALL_WARNINGS)

Expand Down
8 changes: 0 additions & 8 deletions include/xgboost/gbm.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,6 @@ class GradientBooster : public Model, public Configurable {
GradientBooster* /*out*/, bool* /*out_of_bound*/) const {
LOG(FATAL) << "Slice is not supported by current booster.";
}
/*!
* \brief whether the model allow lazy checkpoint
* return true if model is only updated in DoBoost
* after all Allreduce calls
*/
virtual bool AllowLazyCheckPoint() const {
return false;
}
/*! \brief Return number of boosted rounds.
*/
virtual int32_t BoostedRounds() const = 0;
Expand Down
4 changes: 0 additions & 4 deletions include/xgboost/learner.h
Original file line number Diff line number Diff line change
Expand Up @@ -241,10 +241,6 @@ class Learner : public Model, public Configurable, public dmlc::Serializable {
*/
virtual void GetFeatureTypes(std::vector<std::string>* ft) const = 0;

/*!
* \return whether the model allow lazy checkpoint in rabit.
*/
bool AllowLazyCheckPoint() const;
/*!
* \brief Slice the model.
*
Expand Down
6 changes: 2 additions & 4 deletions plugin/example/custom_obj.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,8 @@ class MyLogistic : public ObjFunction {

ObjInfo Task() const override { return ObjInfo::kRegression; }

void GetGradient(const HostDeviceVector<bst_float> &preds,
const MetaInfo &info,
int iter,
HostDeviceVector<GradientPair> *out_gpair) override {
void GetGradient(const HostDeviceVector<bst_float>& preds, const MetaInfo& info, int32_t /*iter*/,
HostDeviceVector<GradientPair>* out_gpair) override {
out_gpair->Resize(preds.Size());
const std::vector<bst_float>& preds_h = preds.HostVector();
std::vector<GradientPair>& out_gpair_h = out_gpair->HostVector();
Expand Down
32 changes: 0 additions & 32 deletions rabit/include/rabit/c_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,38 +135,6 @@ RABIT_DLL int RabitAllreduce(void *sendrecvbuf, size_t count, int enum_dtype,
int enum_op, void (*prepare_fun)(void *arg),
void *prepare_arg);

/*!
* \brief load latest check point
* \param out_global_model hold output of serialized global_model
* \param out_global_len the output length of serialized global model
* \param out_local_model hold output of serialized local_model, can be NULL
* \param out_local_len the output length of serialized local model, can be NULL
*
* \return the version number of check point loaded
* if returned version == 0, this means no model has been CheckPointed
* nothing will be touched
*/
RABIT_DLL int RabitLoadCheckPoint(char **out_global_model,
rbt_ulong *out_global_len,
char **out_local_model,
rbt_ulong *out_local_len);
/*!
* \brief checkpoint the model, meaning we finished a stage of execution
* every time we call check point, there is a version number which will increase by one
*
* \param global_model hold content of serialized global_model
* \param global_len the content length of serialized global model
* \param local_model hold content of serialized local_model, can be NULL
* \param local_len the content length of serialized local model, can be NULL
*
* NOTE: local_model requires explicit replication of the model for fault-tolerance, which will
* bring replication cost in CheckPoint function. global_model do not need explicit replication.
* So only CheckPoint with global_model if possible
*/
RABIT_DLL void RabitCheckPoint(const char *global_model,
rbt_ulong global_len,
const char *local_model,
rbt_ulong local_len);
/*!
* \return version number of current stored model,
* which means how many calls to CheckPoint we made so far
Expand Down
63 changes: 4 additions & 59 deletions rabit/include/rabit/internal/engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,68 +87,13 @@ class IEngine {
*/
virtual void Broadcast(void *sendrecvbuf_, size_t size, int root) = 0;
/*!
* \brief loads the latest check point
* \param global_model pointer to the globally shared model/state
* when calling this function, the caller needs to guarantee that the global_model
* is the same in all nodes
* \param local_model pointer to the local model that is specific to current node/rank
* this can be NULL when no local model is needed
*
* \return the version number of the model loaded
* if returned version == 0, this means no model has been CheckPointed
* the p_model is not touched, users should do necessary initialization by themselves
*
* Common usage example:
* int iter = rabit::LoadCheckPoint(&model);
* if (iter == 0) model.InitParameters();
* for (i = iter; i < max_iter; ++i) {
* do many things, include allreduce
* rabit::CheckPoint(model);
* }
*
* \sa CheckPoint, VersionNumber
* deprecated
*/
virtual int LoadCheckPoint(Serializable *global_model,
Serializable *local_model = nullptr) = 0;
virtual int LoadCheckPoint() = 0;
/*!
* \brief checkpoints the model, meaning a stage of execution was finished
* every time we call check point, a version number increases by ones
*
* \param global_model pointer to the globally shared model/state
* when calling this function, the caller needs to guarantee that the global_model
* is the same in every node
* \param local_model pointer to the local model that is specific to current node/rank
* this can be NULL when no local state is needed
*
* NOTE: local_model requires explicit replication of the model for fault-tolerance, which will
* bring replication cost in CheckPoint function. global_model does not need explicit replication.
* So, only CheckPoint with global_model if possible
*
* \sa LoadCheckPoint, VersionNumber
*/
virtual void CheckPoint(const Serializable *global_model,
const Serializable *local_model = nullptr) = 0;
/*!
* \brief This function can be used to replace CheckPoint for global_model only,
* when certain condition is met (see detailed explanation).
*
* This is a "lazy" checkpoint such that only the pointer to global_model is
* remembered and no memory copy is taken. To use this function, the user MUST ensure that:
* The global_model must remain unchanged until the last call of Allreduce/Broadcast in the current version finishes.
* In other words, global_model can be changed only between the last call of
* Allreduce/Broadcast and LazyCheckPoint in the current version
*
* For example, suppose the calling sequence is:
* LazyCheckPoint, code1, Allreduce, code2, Broadcast, code3, LazyCheckPoint
*
* If the user can only change global_model in code3, then LazyCheckPoint can be used to
* improve the efficiency of the program.
* \param global_model pointer to the globally shared model/state
* when calling this function, the caller needs to guarantee that global_model
* is the same in every node
* \sa LoadCheckPoint, CheckPoint, VersionNumber
* \brief Increase internal version number. Deprecated.
*/
virtual void LazyCheckPoint(const Serializable *global_model) = 0;
virtual void CheckPoint() = 0;
/*!
* \return version number of the current stored model,
* which means how many calls to CheckPoint we made so far
Expand Down
27 changes: 9 additions & 18 deletions rabit/include/rabit/internal/rabit-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ struct BitOR {
dst |= src;
}
};
template<typename OP, typename DType>
inline void Reducer(const void *src_, void *dst_, int len, const MPI::Datatype &dtype) {
const DType* src = static_cast<const DType*>(src_);
DType* dst = (DType*)dst_; // NOLINT(*)
template <typename OP, typename DType>
inline void Reducer(const void *src_, void *dst_, int len, const MPI::Datatype &) {
const DType *src = static_cast<const DType *>(src_);
DType *dst = (DType *)dst_; // NOLINT(*)
for (int i = 0; i < len; i++) {
OP::Reduce(dst[i], src[i]);
}
Expand Down Expand Up @@ -207,20 +207,11 @@ inline void TrackerPrintf(const char *fmt, ...) {
}

#endif // RABIT_STRICT_CXX98_
// load latest check point
inline int LoadCheckPoint(Serializable *global_model,
Serializable *local_model) {
return engine::GetEngine()->LoadCheckPoint(global_model, local_model);
}
// checkpoint the model, meaning we finished a stage of execution
inline void CheckPoint(const Serializable *global_model,
const Serializable *local_model) {
engine::GetEngine()->CheckPoint(global_model, local_model);
}
// lazy checkpoint the model, only remember the pointer to global_model
inline void LazyCheckPoint(const Serializable *global_model) {
engine::GetEngine()->LazyCheckPoint(global_model);
}

// deprecated, planned for removal after checkpoing from JVM package is removed.
inline int LoadCheckPoint() { return engine::GetEngine()->LoadCheckPoint(); }
// deprecated, increase internal version number
inline void CheckPoint() { engine::GetEngine()->CheckPoint(); }
// return the version number of currently stored model
inline int VersionNumber() {
return engine::GetEngine()->VersionNumber();
Expand Down
2 changes: 1 addition & 1 deletion rabit/include/rabit/internal/socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ class TCPSocket : public Socket{
*/
inline void Create(int af = PF_INET) {
#if !IS_MINGW()
sockfd = socket(PF_INET, SOCK_STREAM, 0);
sockfd = socket(af, SOCK_STREAM, 0);
if (sockfd == kInvalidSocket) {
Socket::Error("Create");
}
Expand Down
65 changes: 6 additions & 59 deletions rabit/include/rabit/rabit.h
Original file line number Diff line number Diff line change
Expand Up @@ -205,69 +205,16 @@ template<typename OP, typename DType>
inline void Allreduce(DType *sendrecvbuf, size_t count,
std::function<void()> prepare_fun);
#endif // C++11

/*!
* \brief loads the latest check point
* \param global_model pointer to the globally shared model/state
* when calling this function, the caller needs to guarantee that the global_model
* is the same in every node
* \param local_model pointer to the local model that is specific to the current node/rank
* this can be NULL when no local model is needed
*
* \return the version number of the check point loaded
* if returned version == 0, this means no model has been CheckPointed
* the p_model is not touched, users should do the necessary initialization by themselves
*
* \code{.cpp}
* // Example usage code of LoadCheckPoint
* int iter = rabit::LoadCheckPoint(&model);
* if (iter == 0) model.InitParameters();
* for (i = iter; i < max_iter; ++i) {
* // do many things, include allreduce
* rabit::CheckPoint(model);
* }
* \endcode
* \sa CheckPoint, VersionNumber
* \brief deprecated, planned for removal after checkpoing from JVM package is removed.
*/
inline int LoadCheckPoint(Serializable *global_model,
Serializable *local_model = nullptr);
/*!
* \brief checkpoints the model, meaning a stage of execution has finished.
* every time we call check point, a version number will be increased by one
*
* \param global_model pointer to the globally shared model/state
* when calling this function, the caller needs to guarantee that the global_model
* is the same in every node
* \param local_model pointer to the local model that is specific to the current node/rank
* this can be NULL when no local state is needed
* NOTE: local_model requires explicit replication of the model for fault-tolerance, which will
* bring replication cost in the CheckPoint function. global_model does not need explicit replication.
* So, only CheckPoint with the global_model if possible
* \sa LoadCheckPoint, VersionNumber
*/
inline void CheckPoint(const Serializable *global_model,
const Serializable *local_model = nullptr);
inline int LoadCheckPoint();
/*!
* \brief This function can be used to replace CheckPoint for global_model only,
* when certain condition is met (see detailed explanation).
*
* This is a "lazy" checkpoint such that only the pointer to the global_model is
* remembered and no memory copy is taken. To use this function, the user MUST ensure that:
* The global_model must remain unchanged until the last call of Allreduce/Broadcast in the current version finishes.
* In other words, the global_model model can be changed only between the last call of
* Allreduce/Broadcast and LazyCheckPoint, both in the same version
*
* For example, suppose the calling sequence is:
* LazyCheckPoint, code1, Allreduce, code2, Broadcast, code3, LazyCheckPoint/(or can be CheckPoint)
*
* Then the user MUST only change the global_model in code3.
*
* The use of LazyCheckPoint instead of CheckPoint will improve the efficiency of the program.
* \param global_model pointer to the globally shared model/state
* when calling this function, the caller needs to guarantee that the global_model
* is the same in every node
* \sa LoadCheckPoint, CheckPoint, VersionNumber
* \brief deprecated, planned for removal after checkpoing from JVM package is removed.
*/
inline void LazyCheckPoint(const Serializable *global_model);
inline void CheckPoint();

/*!
* \return version number of the current stored model,
* which means how many calls to CheckPoint we made so far
Expand Down
71 changes: 5 additions & 66 deletions rabit/src/allreduce_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,74 +144,13 @@ class AllreduceBase : public IEngine {
"Broadcast failed");
}
/*!
* \brief load latest check point
* \param global_model pointer to the globally shared model/state
* when calling this function, the caller need to guarantees that global_model
* is the same in all nodes
* \param local_model pointer to local model, that is specific to current node/rank
* this can be NULL when no local model is needed
*
* \return the version number of check point loaded
* if returned version == 0, this means no model has been CheckPointed
* the p_model is not touched, user should do necessary initialization by themselves
*
* Common usage example:
* int iter = rabit::LoadCheckPoint(&model);
* if (iter == 0) model.InitParameters();
* for (i = iter; i < max_iter; ++i) {
* do many things, include allreduce
* rabit::CheckPoint(model);
* }
*
* \brief deprecated
* \sa CheckPoint, VersionNumber
*/
int LoadCheckPoint(Serializable *global_model,
Serializable *local_model = nullptr) override {
return 0;
}
/*!
* \brief checkpoint the model, meaning we finished a stage of execution
* every time we call check point, there is a version number which will increase by one
*
* \param global_model pointer to the globally shared model/state
* when calling this function, the caller need to guarantees that global_model
* is the same in all nodes
* \param local_model pointer to local model, that is specific to current node/rank
* this can be NULL when no local state is needed
*
* NOTE: local_model requires explicit replication of the model for fault-tolerance, which will
* bring replication cost in CheckPoint function. global_model do not need explicit replication.
* So only CheckPoint with global_model if possible
*
* \sa LoadCheckPoint, VersionNumber
*/
void CheckPoint(const Serializable *global_model,
const Serializable *local_model = nullptr) override {
version_number += 1;
}
/*!
* \brief This function can be used to replace CheckPoint for global_model only,
* when certain condition is met(see detailed explanation).
*
* This is a "lazy" checkpoint such that only the pointer to global_model is
* remembered and no memory copy is taken. To use this function, the user MUST ensure that:
* The global_model must remain unchanged until the last call of Allreduce/Broadcast in current version finishes.
* In another words, global_model model can be changed only between last call of
* Allreduce/Broadcast and LazyCheckPoint in current version
*
* For example, suppose the calling sequence is:
* LazyCheckPoint, code1, Allreduce, code2, Broadcast, code3, LazyCheckPoint
*
* If user can only changes global_model in code3, then LazyCheckPoint can be used to
* improve efficiency of the program.
* \param global_model pointer to the globally shared model/state
* when calling this function, the caller need to guarantees that global_model
* is the same in all nodes
* \sa LoadCheckPoint, CheckPoint, VersionNumber
*/
void LazyCheckPoint(const Serializable *global_model) override {
version_number += 1;
}
int LoadCheckPoint() override { return 0; }

// deprecated, increase internal version number
void CheckPoint() override { version_number += 1; }
/*!
* \return version number of current stored model,
* which means how many calls to CheckPoint we made so far
Expand Down

0 comments on commit 142a208

Please sign in to comment.