-
Notifications
You must be signed in to change notification settings - Fork 25
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
New hydro remix algorithm #2532
base: develop
Are you sure you want to change the base?
Conversation
BOOST_CHECK(new_H == expected_H); | ||
BOOST_CHECK(new_D == expected_D); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
J'ai l'impression que ce test est très similaire au test précédent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oui, en effet, la seule chose qui change, c'est la Pmax qui est maintenant de 50 (au lieu de 40).
D'ou le titre de ce 2eme test
std::vector<double> expected_D = {20., 30., 40., 50., 60.}; | ||
BOOST_CHECK(new_H == expected_H); | ||
BOOST_CHECK(new_D == expected_D); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lui aussi ressemble beaucoup au précédent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oui, ce test est le symétrique du test hydro_increases_and_pmax_40mwh___H_is_flattened_to_mean_H_20mwh
C'est pour montrer que l'algo est lui-même symétrique
…acity on remix hydro algorithm
…tial level too low
…fluence on algorithm
int find_min_index(const std::vector<double>& G_plus_H, | ||
const std::vector<double>& new_D, | ||
const std::vector<double>& new_H, | ||
const std::vector<int>& tried_creux, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unclear naming
const std::vector<int>& tried_creux, | ||
const std::vector<double>& P_max, | ||
const std::vector<bool>& filter_hours_remix, | ||
double top) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unclear naming
static bool isLessThan(const std::vector<double>& a, const std::vector<double>& b) | ||
{ | ||
std::vector<double> a_minus_b; | ||
std::ranges::transform(a, b, std::back_inserter(a_minus_b), std::minus<double>()); | ||
return std::ranges::all_of(a_minus_b, [](const double& e) { return e <= 0.; }); | ||
} | ||
|
||
static bool isLessThan(const std::vector<double>& v, const double c) | ||
{ | ||
return std::ranges::all_of(v, [&c](const double& e) { return e <= c; }); | ||
} | ||
|
||
static bool isGreaterThan(const std::vector<double>& v, const double c) | ||
{ | ||
return std::ranges::all_of(v, [&c](const double& e) { return e >= c; }); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use operator overloading, I wrote this example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
static void checkInputCorrectness(const std::vector<double>& G, | ||
const std::vector<double>& H, | ||
const std::vector<double>& D, | ||
const std::vector<double>& levels, | ||
const std::vector<double>& P_max, | ||
const std::vector<double>& P_min, | ||
double initial_level, | ||
double capacity, | ||
const std::vector<double>& inflows, | ||
const std::vector<double>& overflow, | ||
const std::vector<double>& pump, | ||
const std::vector<double>& S, | ||
const std::vector<double>& DTG_MRG) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I don't really agree with having this function
static
, it might be public to ease testing - Please avoid single character variable names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- If this function was a method of a class, it would be private.
Question is : do we expose it to the outside world ?
Well literature says : people should test classes / objects behaviors, not their methods, and especially their private methods (see here for instance). We can't make every private methods public just to test them. So here, we test how the algorithm reacts to incorrect input data. - I do agree
S.size(), | ||
DTG_MRG.size()}; | ||
|
||
if (std::ranges::adjacent_find(sizes, std::not_equal_to()) != sizes.end()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check all sizes against 1st one
if (std::ranges::adjacent_find(sizes, std::not_equal_to()) != sizes.end()) | |
if (!std::ranges::all_of(sizes, [&](std::size_t s){ s == sizes.front();}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Quality Gate failedFailed conditions See analysis details on SonarQube Cloud Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE |
No description provided.