diff --git a/ortools/flatzinc/model.cc b/ortools/flatzinc/model.cc index 91fd65fbb6..a202848d7b 100644 --- a/ortools/flatzinc/model.cc +++ b/ortools/flatzinc/model.cc @@ -22,6 +22,7 @@ #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/strings/str_join.h" +#include "absl/strings/string_view.h" #include "ortools/base/map_util.h" #include "ortools/base/stl_util.h" #include "ortools/util/logging.h" @@ -761,7 +762,7 @@ int Argument::Size() const { // ----- Variable ----- -Variable::Variable(const std::string& name_, const Domain& domain_, +Variable::Variable(absl::string_view name_, const Domain& domain_, bool temporary_) : name(name_), domain(domain_), temporary(temporary_), active(true) { if (!domain.is_interval) { @@ -769,7 +770,7 @@ Variable::Variable(const std::string& name_, const Domain& domain_, } } -bool Variable::Merge(const std::string& other_name, const Domain& other_domain, +bool Variable::Merge(absl::string_view other_name, const Domain& other_domain, bool other_temporary) { if (temporary && !other_temporary) { temporary = false; @@ -834,7 +835,7 @@ Annotation Annotation::AnnotationList(std::vector list) { return result; } -Annotation Annotation::Identifier(const std::string& id) { +Annotation Annotation::Identifier(absl::string_view id) { Annotation result; result.type = IDENTIFIER; result.interval_min = 0; @@ -843,7 +844,7 @@ Annotation Annotation::Identifier(const std::string& id) { return result; } -Annotation Annotation::FunctionCallWithArguments(const std::string& id, +Annotation Annotation::FunctionCallWithArguments(absl::string_view id, std::vector args) { Annotation result; result.type = FUNCTION_CALL; @@ -854,7 +855,7 @@ Annotation Annotation::FunctionCallWithArguments(const std::string& id, return result; } -Annotation Annotation::FunctionCall(const std::string& id) { +Annotation Annotation::FunctionCall(absl::string_view id) { Annotation result; result.type = FUNCTION_CALL; result.interval_min = 0; @@ -896,7 +897,7 @@ Annotation Annotation::VarRefArray(std::vector variables) { return result; } -Annotation Annotation::String(const std::string& str) { +Annotation Annotation::String(absl::string_view str) { Annotation result; result.type = STRING_VALUE; result.interval_min = 0; @@ -957,7 +958,7 @@ std::string SolutionOutputSpecs::Bounds::DebugString() const { } SolutionOutputSpecs SolutionOutputSpecs::SingleVariable( - const std::string& name, Variable* variable, bool display_as_boolean) { + absl::string_view name, Variable* variable, bool display_as_boolean) { SolutionOutputSpecs result; result.name = name; result.variable = variable; @@ -966,7 +967,7 @@ SolutionOutputSpecs SolutionOutputSpecs::SingleVariable( } SolutionOutputSpecs SolutionOutputSpecs::MultiDimensionalArray( - const std::string& name, std::vector bounds, + absl::string_view name, std::vector bounds, std::vector flat_variables, bool display_as_boolean) { SolutionOutputSpecs result; result.variable = nullptr; @@ -1001,7 +1002,7 @@ Model::~Model() { gtl::STLDeleteElements(&constraints_); } -Variable* Model::AddVariable(const std::string& name, const Domain& domain, +Variable* Model::AddVariable(absl::string_view name, const Domain& domain, bool defined) { Variable* const var = new Variable(name, domain, defined); variables_.push_back(var); @@ -1023,8 +1024,8 @@ Variable* Model::AddFloatConstant(double value) { return var; } -void Model::AddConstraint(const std::string& id, - std::vector arguments, bool is_domain) { +void Model::AddConstraint(absl::string_view id, std::vector arguments, + bool is_domain) { Constraint* const constraint = new Constraint(id, std::move(arguments), is_domain); constraints_.push_back(constraint); diff --git a/ortools/flatzinc/model.h b/ortools/flatzinc/model.h index acb0ea8414..ed6d163977 100644 --- a/ortools/flatzinc/model.h +++ b/ortools/flatzinc/model.h @@ -19,6 +19,7 @@ #include #include "absl/container/flat_hash_map.h" +#include "absl/strings/string_view.h" #include "ortools/base/integral_types.h" #include "ortools/base/logging.h" #include "ortools/graph/iterators.h" @@ -127,7 +128,7 @@ struct Variable { // - if one variable is temporary, the name is the name of the other // variable. If both variables are temporary or both variables are not // temporary, the name is chosen arbitrarily between the two names. - bool Merge(const std::string& other_name, const Domain& other_domain, + bool Merge(absl::string_view other_name, const Domain& other_domain, bool other_temporary); std::string DebugString() const; @@ -146,7 +147,7 @@ struct Variable { private: friend class Model; - Variable(const std::string& name_, const Domain& domain_, bool temporary_); + Variable(absl::string_view name_, const Domain& domain_, bool temporary_); }; // An argument is either an integer value, an integer domain, a @@ -216,7 +217,7 @@ struct Argument { // A constraint has a type, some arguments, and a few tags. Typically, a // Constraint is on the heap, and owned by the global Model object. struct Constraint { - Constraint(const std::string& t, std::vector args, + Constraint(absl::string_view t, std::vector args, bool strong_propag) : type(t), arguments(std::move(args)), @@ -271,18 +272,18 @@ struct Annotation { static Annotation Empty(); static Annotation AnnotationList(std::vector list); - static Annotation Identifier(const std::string& id); - static Annotation FunctionCallWithArguments(const std::string& id, + static Annotation Identifier(absl::string_view id); + static Annotation FunctionCallWithArguments(absl::string_view id, std::vector args); - static Annotation FunctionCall(const std::string& id); + static Annotation FunctionCall(absl::string_view id); static Annotation Interval(int64_t interval_min, int64_t interval_max); static Annotation IntegerValue(int64_t value); static Annotation VarRef(Variable* const var); static Annotation VarRefArray(std::vector variables); - static Annotation String(const std::string& str); + static Annotation String(absl::string_view str); std::string DebugString() const; - bool IsFunctionCallWithIdentifier(const std::string& identifier) const { + bool IsFunctionCallWithIdentifier(absl::string_view identifier) const { return type == FUNCTION_CALL && id == identifier; } // Copy all the variable references contained in this annotation (and its @@ -311,14 +312,14 @@ struct SolutionOutputSpecs { }; // Will output: name = . - static SolutionOutputSpecs SingleVariable(const std::string& name, + static SolutionOutputSpecs SingleVariable(absl::string_view name, Variable* variable, bool display_as_boolean); // Will output (for example): // name = array2d(min1..max1, min2..max2, [list of variable values]) // for a 2d array (bounds.size() == 2). static SolutionOutputSpecs MultiDimensionalArray( - const std::string& name, std::vector bounds, + absl::string_view name, std::vector bounds, std::vector flat_variables, bool display_as_boolean); // Empty output. static SolutionOutputSpecs VoidOutput(); @@ -336,7 +337,7 @@ struct SolutionOutputSpecs { class Model { public: - explicit Model(const std::string& name) + explicit Model(absl::string_view name) : name_(name), objective_(nullptr), maximize_(true) {} ~Model(); @@ -344,12 +345,12 @@ class Model { // The objects returned by AddVariable(), AddConstant(), and AddConstraint() // are owned by the model and will remain live for its lifetime. - Variable* AddVariable(const std::string& name, const Domain& domain, + Variable* AddVariable(absl::string_view name, const Domain& domain, bool defined); Variable* AddConstant(int64_t value); Variable* AddFloatConstant(double value); // Creates and add a constraint to the model. - void AddConstraint(const std::string& id, std::vector arguments, + void AddConstraint(absl::string_view id, std::vector arguments, bool is_domain); void AddConstraint(const std::string& id, std::vector arguments); void AddOutput(SolutionOutputSpecs output); diff --git a/ortools/gscip/BUILD.bazel b/ortools/gscip/BUILD.bazel index 2cab5d8c16..efecd8d8da 100644 --- a/ortools/gscip/BUILD.bazel +++ b/ortools/gscip/BUILD.bazel @@ -68,6 +68,7 @@ cc_library( "//ortools/linear_solver:scip_helper_macros", "//ortools/linear_solver:scip_with_glop", "//ortools/port:proto_utils", + "//ortools/util:status_macros", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/container:flat_hash_set", "@com_google_absl//absl/status", diff --git a/ortools/gscip/gscip.cc b/ortools/gscip/gscip.cc index 742898ce05..98acd0ac38 100644 --- a/ortools/gscip/gscip.cc +++ b/ortools/gscip/gscip.cc @@ -40,6 +40,7 @@ #include "ortools/gscip/legacy_scip_params.h" #include "ortools/linear_solver/scip_helper_macros.h" #include "ortools/port/proto_utils.h" +#include "ortools/util/status_macros.h" #include "scip/cons_linear.h" #include "scip/cons_quadratic.h" #include "scip/scip.h" @@ -326,8 +327,8 @@ absl::StatusOr GScip::AddVariable( double lb, double ub, double obj_coef, GScipVarType var_type, const std::string& var_name, const GScipVariableOptions& options) { SCIP_VAR* var = nullptr; - lb = ScipInfClamp(lb); - ub = ScipInfClamp(ub); + OR_ASSIGN_OR_RETURN3(lb, ScipInfClamp(lb), _ << "invalid lower bound"); + OR_ASSIGN_OR_RETURN3(ub, ScipInfClamp(ub), _ << "invalid upper bound"); RETURN_IF_SCIP_ERROR(SCIPcreateVarBasic(scip_, /*var=*/&var, /*name=*/var_name.c_str(), /*lb=*/lb, /*ub=*/ub, @@ -360,11 +361,15 @@ absl::StatusOr GScip::AddLinearConstraint( SCIP_CONS* constraint = nullptr; RETURN_ERROR_UNLESS(range.variables.size() == range.coefficients.size()) << "Error adding constraint: " << name << "."; + OR_ASSIGN_OR_RETURN3(const double lb, ScipInfClamp(range.lower_bound), + _ << "invalid lower bound"); + OR_ASSIGN_OR_RETURN3(const double ub, ScipInfClamp(range.upper_bound), + _ << "invalid upper bound"); RETURN_IF_SCIP_ERROR(SCIPcreateConsLinear( scip_, &constraint, name.c_str(), range.variables.size(), const_cast(range.variables.data()), const_cast(range.coefficients.data()), - ScipInfClamp(range.lower_bound), ScipInfClamp(range.upper_bound), + /*lhs=*/lb, /*rhs=*/ub, /*initial=*/options.initial, /*separate=*/options.separate, /*enforce=*/options.enforce, @@ -392,6 +397,10 @@ absl::StatusOr GScip::AddQuadraticConstraint( << "Error adding quadratic constraint: " << name << " in quadratic term."; RETURN_ERROR_UNLESS(num_quad_vars == range.quadratic_coefficients.size()) << "Error adding quadratic constraint: " << name << " in quadratic term."; + OR_ASSIGN_OR_RETURN3(const double lb, ScipInfClamp(range.lower_bound), + _ << "invalid lower bound"); + OR_ASSIGN_OR_RETURN3(const double ub, ScipInfClamp(range.upper_bound), + _ << "invalid upper bound"); RETURN_IF_SCIP_ERROR(SCIPcreateConsQuadratic( scip_, &constraint, name.c_str(), num_lin_vars, const_cast(range.linear_variables.data()), @@ -399,7 +408,7 @@ absl::StatusOr GScip::AddQuadraticConstraint( const_cast(range.quadratic_variables1.data()), const_cast(range.quadratic_variables2.data()), const_cast(range.quadratic_coefficients.data()), - ScipInfClamp(range.lower_bound), ScipInfClamp(range.upper_bound), + /*lhs=*/lb, /*rhs=*/ub, /*initial=*/options.initial, /*separate=*/options.separate, /*enforce=*/options.enforce, @@ -428,12 +437,15 @@ absl::StatusOr GScip::AddIndicatorConstraint( RETURN_ERROR_UNLESS(indicator_constraint.variables.size() == indicator_constraint.coefficients.size()) << "Error adding indicator constraint: " << name << "."; + OR_ASSIGN_OR_RETURN3(const double ub, + ScipInfClamp(indicator_constraint.upper_bound), + _ << "invalid upper bound"); RETURN_IF_SCIP_ERROR(SCIPcreateConsIndicator( scip_, &constraint, name.c_str(), indicator, indicator_constraint.variables.size(), const_cast(indicator_constraint.variables.data()), const_cast(indicator_constraint.coefficients.data()), - ScipInfClamp(indicator_constraint.upper_bound), + /*rhs=*/ub, /*initial=*/options.initial, /*separate=*/options.separate, /*enforce=*/options.enforce, @@ -598,13 +610,13 @@ absl::Status GScip::SetBranchingPriority(SCIP_VAR* var, int priority) { } absl::Status GScip::SetLb(SCIP_VAR* var, double lb) { - lb = ScipInfClamp(lb); + OR_ASSIGN_OR_RETURN3(lb, ScipInfClamp(lb), _ << "invalid lower bound"); RETURN_IF_SCIP_ERROR(SCIPchgVarLb(scip_, var, lb)); return absl::OkStatus(); } absl::Status GScip::SetUb(SCIP_VAR* var, double ub) { - ub = ScipInfClamp(ub); + OR_ASSIGN_OR_RETURN3(ub, ScipInfClamp(ub), _ << "invalid upper bound"); RETURN_IF_SCIP_ERROR(SCIPchgVarUb(scip_, var, ub)); return absl::OkStatus(); } @@ -711,13 +723,13 @@ absl::string_view GScip::Name(SCIP_CONS* constraint) { } absl::Status GScip::SetLinearConstraintLb(SCIP_CONS* constraint, double lb) { - lb = ScipInfClamp(lb); + OR_ASSIGN_OR_RETURN3(lb, ScipInfClamp(lb), _ << "invalid lower bound"); RETURN_IF_SCIP_ERROR(SCIPchgLhsLinear(scip_, constraint, lb)); return absl::OkStatus(); } absl::Status GScip::SetLinearConstraintUb(SCIP_CONS* constraint, double ub) { - ub = ScipInfClamp(ub); + OR_ASSIGN_OR_RETURN3(ub, ScipInfClamp(ub), _ << "invalid upper bound"); RETURN_IF_SCIP_ERROR(SCIPchgRhsLinear(scip_, constraint, ub)); return absl::OkStatus(); } @@ -810,8 +822,10 @@ absl::StatusOr GScip::Solve( scip_, params.scip_model_filename().c_str(), "cip", FALSE)); } if (params.has_objective_limit()) { - RETURN_IF_SCIP_ERROR( - SCIPsetObjlimit(scip_, ScipInfClamp(params.objective_limit()))); + OR_ASSIGN_OR_RETURN3(const double scip_obj_limit, + ScipInfClamp(params.objective_limit()), + _ << "invalid objective_limit"); + RETURN_IF_SCIP_ERROR(SCIPsetObjlimit(scip_, scip_obj_limit)); } // Install the message handler if necessary. We do this after setting the @@ -986,10 +1000,20 @@ absl::StatusOr GScip::DefaultStringParamValue( return std::string(result); } -double GScip::ScipInfClamp(double d) { +absl::StatusOr GScip::ScipInfClamp(const double d) { const double kScipInf = ScipInf(); - if (d > kScipInf) return kScipInf; - if (d < -kScipInf) return -kScipInf; + if (d == std::numeric_limits::infinity()) { + return kScipInf; + } + if (d == -std::numeric_limits::infinity()) { + return -kScipInf; + } + // NaN is considered finite here. + if (d >= kScipInf || d <= -kScipInf) { + return util::InvalidArgumentErrorBuilder() + << d << " is not in SCIP's finite range: (" << -kScipInf << ", " + << kScipInf << ")"; + } return d; } diff --git a/ortools/gscip/gscip.h b/ortools/gscip/gscip.h index 7ed85e43e6..8c8e194217 100644 --- a/ortools/gscip/gscip.h +++ b/ortools/gscip/gscip.h @@ -23,8 +23,8 @@ // problem creation are thus not supported. // * gSCIP uses std::numeric_limits::infinity(), rather than SCIPs // infinity (a default value of 1e20). Doubles with absolute value >= 1e20 -// are automatically converting to std::numeric_limits::infinity() -// by gSCIP. Changing the underlying SCIP's infinity is not supported. +// but < inf result in an error. Changing the underlying SCIP's infinity is +// not supported. // * absl::Status and absl::StatusOr are used to propagate SCIP errors (and on // a best effort basis, also filter out bad input to gSCIP functions). // @@ -324,10 +324,11 @@ class GScip { // more useful. absl::Status SetBranchingPriority(SCIP_VAR* var, int priority); - // Doubles with absolute value of at least this value are replaced by this - // value before giving them SCIP. SCIP considers values at least this large to - // be infinite. When querying gSCIP, if an absolute value exceeds ScipInf, it - // is replaced by std::numeric_limits::infinity(). + // Doubles with absolute value of at least this value are invalid and result + // in errors. Floating point actual infinities are replaced by this value in + // SCIP calls. SCIP considers values at least this large to be infinite. When + // querying gSCIP, if an absolute value exceeds ScipInf, it is replaced by + // std::numeric_limits::infinity(). double ScipInf(); static constexpr double kDefaultScipInf = 1e20; @@ -359,8 +360,8 @@ class GScip { absl::Status SetParams(const GScipParameters& params, const std::string& legacy_params); absl::Status FreeTransform(); - // Clamps d to [-ScipInf(), ScipInf()]. - double ScipInfClamp(double d); + // Replaces +/- inf by +/- ScipInf(), fails when |d| is in [ScipInf(), inf). + absl::StatusOr ScipInfClamp(double d); // Returns +/- inf if |d| >= ScipInf(), otherwise returns d. double ScipInfUnclamp(double d);