-
Notifications
You must be signed in to change notification settings - Fork 141
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
issue1088 #156
issue1088 #156
Changes from 2 commits
1c7edd2
e872120
85c62cc
c8a43a1
c29ae0f
215b410
40b20f2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,11 +32,20 @@ | |
#include <cassert> | ||
#include <iostream> | ||
#include <numeric> | ||
#include <unordered_set> | ||
|
||
using namespace std; | ||
using utils::ExitCode; | ||
|
||
namespace lp { | ||
#ifndef NDEBUG | ||
template<class T> | ||
static bool all_values_are_unique(const vector<T> &v) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move this function next to sorted_unique? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not familiar enough with our conventions here. Moving it to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have no strong opinion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You currently also have an additional import here (for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, I overlooked that because I first used The disadvantage of defining it locally is that somebody else might define something similar locally unaware of this function here. So I'll just move it to |
||
unordered_set<T> s(v.begin(), v.end()); | ||
return s.size() == v.size(); | ||
} | ||
#endif | ||
|
||
void add_lp_solver_option_to_feature(plugins::Feature &feature) { | ||
feature.add_option<LPSolverType>( | ||
"lpsolver", | ||
|
@@ -185,6 +194,7 @@ void LPSolver::load_problem(const LinearProgram &lp) { | |
|
||
for (const LPConstraint &constraint : lp.get_constraints()) { | ||
const vector<int> &vars = constraint.get_variables(); | ||
assert(all_values_are_unique(vars)); | ||
const vector<double> &coeffs = constraint.get_coefficients(); | ||
assert(vars.size() == coeffs.size()); | ||
starts.push_back(elements.size()); | ||
|
@@ -272,6 +282,7 @@ void LPSolver::add_temporary_constraints(const vector<LPConstraint> &constraints | |
clear_temporary_data(); | ||
int num_rows = constraints.size(); | ||
for (const LPConstraint &constraint : constraints) { | ||
assert(all_values_are_unique(constraint.get_variables())); | ||
row_lb.push_back(constraint.get_lower_bound()); | ||
row_ub.push_back(constraint.get_upper_bound()); | ||
rows.push_back(new CoinShallowPackedVector( | ||
|
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.
Do you get an "unused function" error without this? If not, I'd reduce the number of preprocessor directives and just always define the function (it shouldn't have a performance impact if it is not used). I have no strong opinion, though.
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.
No, I thought that's something we do if something's used only in debug mode. I'll remove it.