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

[ML] Update C++ standard to 20 and refine clang-tidy checks #2777

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -8,32 +8,31 @@ Checks: >

clang-diagnostic-*,
-clang-diagnostic-sign-conversion,
-clang-diagnostic-unused-macros,

google-*,
-google-build-using-namespace,
-google-readability-namespace-comments,
-google-runtime-references,

misc-*,
-misc-non-private-member-variables-in-classes,
-misc-include-cleaner,

modernize-*,
-modernize-use-trailing-return-type,
-modernize-concat-nested-namespaces,
-modernize-use-nodiscard,
-modernize-replace-random-shuffle,
-modernize-unary-static-assert,
-modernize-use-uncaught-exception,

performance-*,

readability-*,
-readability-magic-numbers,
-readability-named-parameter,
-readability-redundant-access-specifiers,
-readability-simplify-boolean-expr,
-readability-identifier-length,

WarningsAsErrors: false
AnalyzeTemporaryDtors: false
FormatStyle: file
CheckOptions:
- key: bugprone-assert-side-effect.AssertMacros
Expand Down Expand Up @@ -61,4 +60,6 @@ CheckOptions:
- key: bugprone-suspicious-string-compare.WarnOnLogicalNotComparison
value: 'true'
- key: readability-function-cognitive-complexity.IgnoreMacros
value: 'true'
value: 'true'
- key: modernize-include.UseAngleBrackets
value: true
2 changes: 1 addition & 1 deletion cmake/variables.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#

# set the C++ standard we need to enforce
set (CMAKE_CXX_STANDARD 17 CACHE STRING "The C++ standard to use")
set (CMAKE_CXX_STANDARD 20 CACHE STRING "The C++ standard to use")
set (CMAKE_CXX_STANDARD_REQUIRED ON)
set (CMAKE_CXX_EXTENSIONS OFF)

Expand Down
28 changes: 15 additions & 13 deletions include/api/CAnomalyJob.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ class API_EXPORT CAnomalyJob : public CDataProcessor {
struct SBackgroundPersistArgs {
SBackgroundPersistArgs(core_t::TTime time,
const model::CResourceMonitor::SModelSizeStats& modelSizeStats,
const model::CInterimBucketCorrector& interimBucketCorrector,
model::CInterimBucketCorrector interimBucketCorrector,
const model::CHierarchicalResultsAggregator& aggregator,
core_t::TTime latestRecordTime,
core_t::TTime lastResultsTime,
Expand All @@ -146,12 +146,12 @@ class API_EXPORT CAnomalyJob : public CDataProcessor {
using TBackgroundPersistArgsPtr = std::shared_ptr<SBackgroundPersistArgs>;

public:
CAnomalyJob(const std::string& jobId,
CAnomalyJob(std::string jobId,
model::CLimits& limits,
CAnomalyJobConfig& jobConfig,
model::CAnomalyDetectorModelConfig& modelConfig,
core::CJsonOutputStreamWrapper& outputBuffer,
const TPersistCompleteFunc& persistCompleteFunc,
TPersistCompleteFunc persistCompleteFunc,
CPersistenceManager* persistenceManager,
core_t::TTime maxQuantileInterval,
const std::string& timeFieldName,
Expand Down Expand Up @@ -265,7 +265,8 @@ class API_EXPORT CAnomalyJob : public CDataProcessor {

//! This is the function that is called in a different thread to the
//! main processing when background persistence is triggered.
bool runBackgroundPersist(TBackgroundPersistArgsPtr args, core::CDataAdder& persister);
bool runBackgroundPersist(const TBackgroundPersistArgsPtr& args,
core::CDataAdder& persister);

//! This function is called from the persistence manager when foreground persistence is triggered
bool runForegroundPersist(core::CDataAdder& persister);
Expand Down Expand Up @@ -331,9 +332,9 @@ class API_EXPORT CAnomalyJob : public CDataProcessor {

//! Parses the time range in a control message assuming the time range follows after a
//! single character code (e.g. starts with 'i10 20').
bool parseTimeRangeInControlMessage(const std::string& controlMessage,
core_t::TTime& start,
core_t::TTime& end);
static bool parseTimeRangeInControlMessage(const std::string& controlMessage,
core_t::TTime& start,
core_t::TTime& end);

//! Update equalizers if not interim and aggregate.
void updateAggregatorAndAggregate(bool isInterim, model::CHierarchicalResults& results);
Expand Down Expand Up @@ -375,24 +376,25 @@ class API_EXPORT CAnomalyJob : public CDataProcessor {
//! Update configuration
void doForecast(const std::string& controlMessage);

TAnomalyDetectorPtr
static TAnomalyDetectorPtr
makeDetector(const model::CAnomalyDetectorModelConfig& modelConfig,
model::CLimits& limits,
const std::string& partitionFieldValue,
core_t::TTime firstTime,
const model::CAnomalyDetector::TModelFactoryCPtr& modelFactory);

//! Populate detector keys from the anomaly job config.
void populateDetectorKeys(const CAnomalyJobConfig& jobConfig, TKeyVec& keys);
static void populateDetectorKeys(const CAnomalyJobConfig& jobConfig, TKeyVec& keys);

//! Extract the field called \p fieldName from \p dataRowFields.
const std::string* fieldValue(const std::string& fieldName, const TStrStrUMap& dataRowFields);
static const std::string* fieldValue(const std::string& fieldName,
const TStrStrUMap& dataRowFields);

//! Extract the required fields from \p dataRowFields
//! and add the new record to \p detector
void addRecord(const TAnomalyDetectorPtr detector,
core_t::TTime time,
const TStrStrUMap& dataRowFields);
static void addRecord(const TAnomalyDetectorPtr& detector,
core_t::TTime time,
const TStrStrUMap& dataRowFields);

//! Parses a control message requesting that model state be persisted.
//! Extracts optional arguments to be used for persistence.
Expand Down
Loading