Skip to content

Commit a8bc9eb

Browse files
foolnotionfoolnotion
authored
refactor: fix clang-tidy warnings
Refactor/clang tidy
2 parents 0c0f116 + e89c518 commit a8bc9eb

65 files changed

Lines changed: 1165 additions & 1147 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.clang-tidy‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,20 @@ Checks: "*,\
1818
-bugprone-easily-swappable-parameters,\
1919
-boost-use-ranges,\
2020
-modernize-use-ranges,\
21-
-misc-non-private-member-variables-in-classes"
21+
-misc-non-private-member-variables-in-classes,\
22+
-cppcoreguidelines-avoid-magic-numbers,\
23+
-readability-magic-numbers,\
24+
-misc-include-cleaner,\
25+
-bugprone-argument-comment,\
26+
-llvm-prefer-static-over-anonymous-namespace,\
27+
-portability-template-virtual-member-function,\
28+
-misc-no-recursion,\
29+
-cppcoreguidelines-avoid-do-while,\
30+
-abseil-string-find-str-contains,\
31+
-bugprone-unchecked-optional-access,\
32+
-concurrency-mt-unsafe,\
33+
-cert-err58-cpp"
34+
HeaderFilterRegex: '^.*/operon/(include/operon/(?!ceres|mdspan)|source|cli|test/source/(?!.*thirdparty))'
2235
WarningsAsErrors: ''
2336
CheckOptions:
2437
- key: 'bugprone-argument-comment.StrictMode'

‎CMakeLists.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# SPDX-License-Identifier: MIT
22
# SPDX-FileCopyrightText: Copyright 2019-2023 Heal Research
33

4-
cmake_minimum_required(VERSION 3.20)
4+
cmake_minimum_required(VERSION 3.25)
55

66
include(cmake/prelude.cmake)
77

@@ -248,8 +248,8 @@ target_link_libraries(operon_operon PUBLIC
248248
lbfgs::lbfgs
249249
libassert::assert
250250
infix-parser::infix-parser # required by infix parser
251-
std::mdspan
252251
vstat::vstat
252+
std::mdspan
253253
)
254254

255255
target_link_libraries(operon_operon PRIVATE

‎cli/source/operator_factory.cpp‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ namespace Operon { struct Variable; }
2424

2525
namespace Operon {
2626

27-
namespace detail {
27+
namespace {
2828
auto GetErrorString(std::string const& name, std::string const& arg) {
2929
return fmt::format("unable to parse {} argument '{}'", name, arg);
3030
}
31-
} // namespace detail
31+
} // namespace
3232

3333
auto ParseReinserter(std::string const& str, ComparisonCallback&& comp) -> std::unique_ptr<ReinserterBase>
3434
{
@@ -38,7 +38,7 @@ auto ParseReinserter(std::string const& str, ComparisonCallback&& comp) -> std::
3838
} else if (str == "replace-worst") {
3939
reinserter = std::make_unique<ReplaceWorstReinserter>(std::move(comp));
4040
} else {
41-
throw std::invalid_argument(detail::GetErrorString("reinserter", str));
41+
throw std::invalid_argument(GetErrorString("reinserter", str));
4242
}
4343
return reinserter;
4444
}
@@ -73,7 +73,7 @@ auto ParseSelector(std::string const& str, ComparisonCallback&& comp) -> std::un
7373
} else if (name == "random") {
7474
selector = std::make_unique<Operon::RandomSelector>();
7575
} else {
76-
throw std::invalid_argument(detail::GetErrorString("selector", str));
76+
throw std::invalid_argument(GetErrorString("selector", str));
7777
}
7878

7979
return selector;
@@ -84,7 +84,6 @@ auto ParseCreator(std::string const& str, PrimitiveSet const& pset, std::vector<
8484
std::unique_ptr<CreatorBase> creator;
8585

8686
auto tok = Split(str, ':');
87-
auto name = tok[0];
8887

8988
double bias{0}; // irregularity bias (used by btc and ptc20)
9089
if(tok.size() > 1) {
@@ -100,7 +99,7 @@ auto ParseCreator(std::string const& str, PrimitiveSet const& pset, std::vector<
10099
} else if (str == "grow") {
101100
creator = std::make_unique<GrowTreeCreator>(&pset, inputs, maxLength);
102101
} else {
103-
throw std::invalid_argument(detail::GetErrorString("creator", str));
102+
throw std::invalid_argument(GetErrorString("creator", str));
104103
}
105104
return creator;
106105
}
@@ -168,7 +167,7 @@ auto ParseGenerator(std::string const& str, EvaluatorBase& eval, CrossoverBase&
168167
if (tok.size() > 1) { polygenicSize = scn::scan<size_t>(tok[1], "{}")->value(); }
169168
dynamic_cast<PolygenicOffspringGenerator*>(generator.get())->PolygenicSize(polygenicSize);
170169
} else {
171-
throw std::invalid_argument(detail::GetErrorString("generator", str));
170+
throw std::invalid_argument(GetErrorString("generator", str));
172171
}
173172
return generator;
174173
}

‎cli/source/operator_factory.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ auto ParseEvaluator(std::string const& str, Problem& problem, ScalarDispatch& dt
4242

4343
auto ParseErrorMetric(std::string const& str) -> std::tuple<std::unique_ptr<Operon::ErrorMetric>, bool>;
4444

45-
auto ParseGenerator(std::string const& str, EvaluatorBase& eval, CrossoverBase& cx, MutatorBase& mut, SelectorBase& femSel, SelectorBase& maleSel, CoefficientOptimizer const* cOpt) -> std::unique_ptr<OffspringGeneratorBase>;
45+
auto ParseGenerator(std::string const& str, EvaluatorBase& eval, CrossoverBase& cx, MutatorBase& mut, SelectorBase& femSel, SelectorBase& maleSel, CoefficientOptimizer const* coeffOptimizer) -> std::unique_ptr<OffspringGeneratorBase>;
4646

4747
auto ParseOptimizer(std::string const& str, Problem const& problem, ScalarDispatch const& dtable) -> std::unique_ptr<OptimizerBase>;
4848

‎cli/source/operon_gp.cpp‎

Lines changed: 52 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,29 @@
3030
#include "operator_factory.hpp"
3131
#include "util.hpp"
3232

33-
auto main(int argc, char** argv) -> int
33+
namespace {
34+
auto MakeCoeffAndMutation(bool symbolic)
35+
-> std::pair<std::unique_ptr<Operon::CoefficientInitializerBase>, std::unique_ptr<Operon::MutatorBase>>
36+
{
37+
if (symbolic) {
38+
using Dist = std::uniform_int_distribution<int>;
39+
auto ci = std::make_unique<Operon::CoefficientInitializer<Dist>>();
40+
int constexpr range { 5 };
41+
dynamic_cast<Operon::CoefficientInitializer<Dist>*>(ci.get())->ParameterizeDistribution(-range, +range);
42+
auto op = std::make_unique<Operon::OnePointMutation<Dist>>();
43+
dynamic_cast<Operon::OnePointMutation<Dist>*>(op.get())->ParameterizeDistribution(-range, +range);
44+
return { std::move(ci), std::move(op) };
45+
}
46+
using Dist = std::normal_distribution<Operon::Scalar>;
47+
auto ci = std::make_unique<Operon::CoefficientInitializer<Dist>>();
48+
dynamic_cast<Operon::NormalCoefficientInitializer*>(ci.get())->ParameterizeDistribution(Operon::Scalar { 0 }, Operon::Scalar { 1 });
49+
auto op = std::make_unique<Operon::OnePointMutation<Dist>>();
50+
dynamic_cast<Operon::OnePointMutation<Dist>*>(op.get())->ParameterizeDistribution(Operon::Scalar { 0 }, Operon::Scalar { 1 });
51+
return { std::move(ci), std::move(op) };
52+
}
53+
} // anonymous namespace
54+
55+
auto main(int argc, char** argv) -> int // NOLINT(bugprone-exception-escape)
3456
{
3557
auto opts = Operon::InitOptions("operon_gp", "Genetic programming symbolic regression");
3658
auto result = Operon::ParseOptions(std::move(opts), argc, argv);
@@ -47,7 +69,6 @@ auto main(int argc, char** argv) -> int
4769
config.TimeLimit = result["timelimit"].as<size_t>();
4870
config.Seed = std::random_device {}();
4971

50-
// parse remaining configuration
5172
Operon::Range trainingRange;
5273
Operon::Range testRange;
5374
std::unique_ptr<Operon::Dataset> dataset;
@@ -58,117 +79,58 @@ auto main(int argc, char** argv) -> int
5879

5980
auto maxLength = result["maxlength"].as<size_t>();
6081
auto maxDepth = result["maxdepth"].as<size_t>();
61-
auto crossoverInternalProbability = result["crossover-internal-probability"].as<Operon::Scalar>();
62-
63-
auto symbolic = result["symbolic"].as<bool>();
82+
auto const crossoverInternalProbability = result["crossover-internal-probability"].as<Operon::Scalar>();
83+
auto const symbolic = result["symbolic"].as<bool>();
84+
85+
// Apply overrides from parsed options
86+
dataset = std::make_unique<Operon::Dataset>(result["dataset"].as<std::string>(), /*hasHeader=*/true);
87+
ENSURE(!dataset->IsView());
88+
if (result.contains("seed")) { config.Seed = result["seed"].as<size_t>(); }
89+
if (result.contains("train")) { trainingRange = Operon::ParseRange(result["train"].as<std::string>()); }
90+
if (result.contains("test")) { testRange = Operon::ParseRange(result["test"].as<std::string>()); }
91+
if (result.contains("target")) { targetName = result["target"].as<std::string>(); }
92+
if (result.contains("enable-symbols")) { primitiveSetConfig |= Operon::ParsePrimitiveSetConfig(result["enable-symbols"].as<std::string>()); }
93+
if (result.contains("disable-symbols")) { primitiveSetConfig &= ~Operon::ParsePrimitiveSetConfig(result["disable-symbols"].as<std::string>()); }
94+
if (result.contains("threads")) { threads = static_cast<decltype(threads)>(result["threads"].as<size_t>()); }
95+
if (result.contains("show-primitives")) { showPrimitiveSet = true; }
6496

6597
try {
66-
for (const auto& kv : result.arguments()) {
67-
const auto& key = kv.key();
68-
const auto& value = kv.value();
69-
70-
if (key == "dataset") {
71-
dataset = std::make_unique<Operon::Dataset>(value, true);
72-
ENSURE(!dataset->IsView());
73-
}
74-
if (key == "seed") {
75-
config.Seed = kv.as<size_t>();
76-
}
77-
if (key == "train") {
78-
trainingRange = Operon::ParseRange(value);
79-
}
80-
if (key == "test") {
81-
testRange = Operon::ParseRange(value);
82-
}
83-
if (key == "target") {
84-
targetName = value;
85-
}
86-
if (key == "maxlength") {
87-
maxLength = kv.as<size_t>();
88-
}
89-
if (key == "maxdepth") {
90-
maxDepth = kv.as<size_t>();
91-
}
92-
if (key == "enable-symbols") {
93-
auto mask = Operon::ParsePrimitiveSetConfig(value);
94-
primitiveSetConfig |= mask;
95-
}
96-
if (key == "disable-symbols") {
97-
auto mask = ~Operon::ParsePrimitiveSetConfig(value);
98-
primitiveSetConfig &= mask;
99-
}
100-
if (key == "threads") {
101-
threads = static_cast<decltype(threads)>(kv.as<size_t>());
102-
}
103-
if (key == "show-primitives") {
104-
showPrimitiveSet = true;
105-
}
106-
}
107-
10898
if (showPrimitiveSet) {
10999
Operon::PrintPrimitives(primitiveSetConfig);
110100
return EXIT_SUCCESS;
111101
}
112102

113103
// set the target
114-
Operon::Variable target;
115104
auto res = dataset->GetVariable(targetName);
116105
if (!res) {
117106
fmt::print(stderr, "error: target variable {} does not exist in the dataset.", targetName);
118107
return EXIT_FAILURE;
119108
}
120-
target = *res;
109+
auto const& target = *res;
121110
auto const rows { dataset->Rows<std::size_t>() };
122-
if (result.count("train") == 0) {
123-
trainingRange = Operon::Range { 0, 2 * rows / 3 }; // by default use 66% of the data as training
124-
}
125-
if (result.count("test") == 0) {
126-
// if no test range is specified, we try to infer a reasonable range based on the trainingRange
127-
if (trainingRange.Start() > 0) {
128-
testRange = Operon::Range { 0, trainingRange.Start() };
129-
} else if (trainingRange.End() < rows) {
130-
testRange = Operon::Range { trainingRange.End(), rows };
131-
} else {
132-
testRange = Operon::Range { 0, 1 };
133-
}
134-
}
111+
112+
Operon::SetupRanges(result, *dataset, trainingRange, testRange);
113+
135114
// validate training range
136115
if (trainingRange.Start() >= rows || trainingRange.End() > rows) {
137116
fmt::print(stderr, "error: the training range {}:{} exceeds the available data range ({} rows)\n", trainingRange.Start(), trainingRange.End(), dataset->Rows());
138117
return EXIT_FAILURE;
139118
}
140-
141119
if (trainingRange.Start() > trainingRange.End()) {
142120
fmt::print(stderr, "error: invalid training range {}:{}\n", trainingRange.Start(), trainingRange.End());
143121
return EXIT_FAILURE;
144122
}
145123

146-
std::vector<Operon::Hash> inputs;
147-
if (result.count("inputs") == 0) {
148-
inputs = dataset->VariableHashes();
149-
std::erase(inputs, target.Hash);
150-
} else {
151-
auto str = result["inputs"].as<std::string>();
152-
auto tokens = Operon::Split(str, ',');
153-
154-
for (auto const& tok : tokens) {
155-
if (auto res = dataset->GetVariable(tok); res.has_value()) {
156-
inputs.push_back(res->Hash);
157-
} else {
158-
fmt::print(stderr, "error: variable {} does not exist in the dataset.", tok);
159-
return EXIT_FAILURE;
160-
}
161-
}
162-
}
124+
auto inputs = Operon::BuildInputs(result, *dataset, target.Hash);
125+
163126
Operon::Problem problem(std::move(dataset));
164127
problem.SetTrainingRange(trainingRange);
165128
problem.SetTestRange(testRange);
166129
problem.SetTarget(target.Hash);
167130
problem.SetInputs(inputs);
168131
problem.ConfigurePrimitiveSet(primitiveSetConfig);
169132

170-
std::unique_ptr<Operon::CreatorBase> creator;
171-
creator = ParseCreator(result["creator"].as<std::string>(), problem.GetPrimitiveSet(), problem.GetInputs(), maxLength);
133+
auto creator = ParseCreator(result["creator"].as<std::string>(), problem.GetPrimitiveSet(), problem.GetInputs(), maxLength);
172134

173135
auto [amin, amax] = problem.GetPrimitiveSet().FunctionArityLimits();
174136
Operon::UniformTreeInitializer treeInitializer(creator.get());
@@ -178,23 +140,8 @@ auto main(int argc, char** argv) -> int
178140
treeInitializer.ParameterizeDistribution(amin + 1, maxLength);
179141
treeInitializer.SetMinDepth(initialMinDepth);
180142
treeInitializer.SetMaxDepth(initialMaxDepth); // NOLINT
181-
//
182-
std::unique_ptr<Operon::CoefficientInitializerBase> coeffInitializer;
183-
std::unique_ptr<Operon::MutatorBase> onePoint;
184-
if (symbolic) {
185-
using Dist = std::uniform_int_distribution<int>;
186-
coeffInitializer = std::make_unique<Operon::CoefficientInitializer<Dist>>();
187-
int constexpr range { 5 };
188-
dynamic_cast<Operon::CoefficientInitializer<Dist>*>(coeffInitializer.get())->ParameterizeDistribution(-range, +range);
189-
onePoint = std::make_unique<Operon::OnePointMutation<Dist>>();
190-
dynamic_cast<Operon::OnePointMutation<Dist>*>(onePoint.get())->ParameterizeDistribution(-range, +range);
191-
} else {
192-
using Dist = std::normal_distribution<Operon::Scalar>;
193-
coeffInitializer = std::make_unique<Operon::CoefficientInitializer<Dist>>();
194-
dynamic_cast<Operon::NormalCoefficientInitializer*>(coeffInitializer.get())->ParameterizeDistribution(Operon::Scalar { 0 }, Operon::Scalar { 1 });
195-
onePoint = std::make_unique<Operon::OnePointMutation<Dist>>();
196-
dynamic_cast<Operon::OnePointMutation<Dist>*>(onePoint.get())->ParameterizeDistribution(Operon::Scalar { 0 }, Operon::Scalar { 1 });
197-
}
143+
144+
auto [coeffInitializer, onePoint] = MakeCoeffAndMutation(symbolic);
198145

199146
Operon::SubtreeCrossover crossover { crossoverInternalProbability, maxDepth, maxLength };
200147
Operon::MultiMutation mutator {};
@@ -217,18 +164,18 @@ auto main(int argc, char** argv) -> int
217164
mutator.Add(&discretePoint, 1.0);
218165

219166
Operon::ScalarDispatch dtable;
220-
auto scale = result["linear-scaling"].as<bool>();
167+
auto const scale = result["linear-scaling"].as<bool>();
221168
auto evaluator = Operon::ParseEvaluator(result["objective"].as<std::string>(), problem, dtable, scale);
222169
evaluator->SetBudget(config.Evaluations);
223170

224171
auto optimizer = std::make_unique<Operon::LevenbergMarquardtOptimizer<decltype(dtable), Operon::OptimizerType::Eigen>>(&dtable, &problem);
225172
optimizer->SetIterations(config.Iterations);
226173

227-
Operon::CoefficientOptimizer cOpt { optimizer.get() };
174+
Operon::CoefficientOptimizer const cOpt { optimizer.get() };
228175

229176
EXPECT(problem.TrainingRange().Size() > 0);
230177

231-
auto comp = [](auto const& lhs, auto const& rhs) { return lhs[0] < rhs[0]; };
178+
auto comp = [](auto const& lhs, auto const& rhs) -> auto { return lhs[0] < rhs[0]; };
232179

233180
auto femaleSelector = Operon::ParseSelector(result["female-selector"].as<std::string>(), comp);
234181
auto maleSelector = Operon::ParseSelector(result["male-selector"].as<std::string>(), comp);
@@ -244,19 +191,15 @@ auto main(int argc, char** argv) -> int
244191
}
245192

246193
Operon::RandomGenerator random(config.Seed);
247-
if (result["shuffle"].as<bool>()) {
248-
problem.GetDataset()->Shuffle(random);
249-
}
250-
if (result["standardize"].as<bool>()) {
251-
problem.StandardizeData(problem.TrainingRange());
252-
}
194+
if (result["shuffle"].as<bool>()) { problem.GetDataset()->Shuffle(random); }
195+
if (result["standardize"].as<bool>()) { problem.StandardizeData(problem.TrainingRange()); }
253196

254197
tf::Executor executor(threads);
255198
Operon::GeneticProgrammingAlgorithm gp { config, &problem, &treeInitializer, coeffInitializer.get(), generator.get(), reinserter.get() };
256199

257200
auto const* ptr = dynamic_cast<Operon::Evaluator<decltype(dtable)> const*>(evaluator.get());
258201
Operon::Reporter<Operon::Evaluator<decltype(dtable)>> reporter(ptr);
259-
gp.Run(executor, random, [&]() { reporter(executor, gp); });
202+
gp.Run(executor, random, [&]() -> void { reporter(executor, gp); });
260203
auto best = reporter.GetBest();
261204
fmt::print("{}\n", Operon::InfixFormatter::Format(best.Genotype, *problem.GetDataset(), 6));
262205
} catch (std::exception& e) {

0 commit comments

Comments
 (0)