Skip to content

Commit 21c386b

Browse files
committed
Removed Python specific error messages and cleaned up HiPO logging in HighsOptions to remove redundancy.
The new error message is also more generic, in case future algorithms need these external dependencies.
1 parent 0be99df commit 21c386b

4 files changed

Lines changed: 106 additions & 113 deletions

File tree

check/TestHighsExternalDeps.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,14 @@ TEST_CASE("HighsExternalDeps-compile-time", "[highs_external_deps]") {
2222
// isAvailableAtCompile should be consistent with build config
2323
bool compile = HighsExternalDeps::isAvailableAtCompile();
2424

25-
// If compile-time available, runtime must also be available
25+
// If compile-time aware and statically linked, it must be available
26+
// If compile-time aware and dynamically linked, it may or may not be available
27+
// at runtime
28+
#if !defined(HIGHS_SHARED_EXTRAS_LIBRARY)
2629
if (compile) {
2730
REQUIRE(HighsExternalDeps::isAvailable());
2831
}
32+
#endif
2933
}
3034

3135
TEST_CASE("HighsExternalDeps-getCopyrightInfo", "[highs_external_deps]") {

highs/HighsExternalDeps.cpp

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,32 @@ bool HighsExternalDeps::tryLoad() {
2929
return tryLoad(empty_path);
3030
}
3131

32+
void HighsExternalDeps::logUnavailable(const HighsLogOptions& log_options,
33+
const HighsLogType type,
34+
const char* format, ...) {
35+
if (!isAvailable()) {
36+
va_list argptr;
37+
va_start(argptr, format);
38+
std::array<char, kIoBufferSize> msgbuffer = {};
39+
int len = vsnprintf(msgbuffer.data(), msgbuffer.size(), format, argptr);
40+
va_end(argptr);
41+
42+
if (!isAvailableAtCompile()) {
43+
highsLogUser(log_options, type, "%s%s%s", msgbuffer.data(),
44+
(len > 0) ? " " : "",
45+
"This build was compiled without external dependencies. "
46+
"Reconfigure with -DHIPO=ON to enable.\n");
47+
} else {
48+
highsLogUser(
49+
log_options, type, "%s%s%s", msgbuffer.data(), (len > 0) ? " " : "",
50+
"The external dependencies are missing. Please install "
51+
"highs_extras library. For example, in Python use `pip install "
52+
"highspy[extras]`. Note, this changes the HiGHS license from MIT "
53+
"to Apache, due to the dependencies' licensing.\n");
54+
}
55+
}
56+
}
57+
3258
#ifdef HIGHS_SHARED_EXTRAS_LIBRARY
3359
// Platform-specific includes for dynamic loading
3460
#if defined(_WIN32) || defined(_WIN64)
@@ -171,9 +197,7 @@ bool HighsExternalDeps::tryLoad(const std::string& path) {
171197
std::string extras_version = get_version();
172198
if (extras_version != highs_version) {
173199
inst.status_ = "Extras: ABI version mismatch: expected " +
174-
highs_version + ", got " + extras_version +
175-
". Please reinstall: pip install --force-reinstall "
176-
"highspy[extras]";
200+
highs_version + ", got " + extras_version + ".";
177201
inst.unload();
178202
ok = false;
179203
} else {

highs/HighsExternalDeps.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <string>
1414

1515
#include "HighsExtrasApi.h"
16+
#include "io/HighsIO.h"
1617
#include "rcm/rcm.h"
1718
#include "util/HighsInt.h"
1819

@@ -199,6 +200,9 @@ struct HighsExternalDeps {
199200
HighsExternalDeps& operator=(const HighsExternalDeps&) = delete;
200201

201202
static bool tryLoad();
203+
static void logUnavailable(const HighsLogOptions& log_options,
204+
const HighsLogType type, const char* format = "",
205+
...);
202206

203207
// Exclude dependencies
204208
#ifndef HIPO
@@ -215,7 +219,7 @@ struct HighsExternalDeps {
215219
// Shared library support
216220
#elif defined(HIGHS_SHARED_EXTRAS_LIBRARY)
217221
static inline bool isAvailable() { return tryLoad(); }
218-
static constexpr bool isAvailableAtCompile() { return false; }
222+
static constexpr bool isAvailableAtCompile() { return true; }
219223

220224
static bool tryLoad(const std::string& path);
221225
static void unload();
@@ -234,7 +238,6 @@ struct HighsExternalDeps {
234238
static inline const std::string getLoadStatus() {
235239
return "Extras: Available at compile time";
236240
}
237-
238241
#endif
239242

240243
private:

highs/lp_data/HighsOptions.cpp

Lines changed: 69 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -81,120 +81,83 @@ bool optionOffOnOk(const HighsLogOptions& report_log_options,
8181

8282
bool optionSolverOk(const HighsLogOptions& report_log_options,
8383
const string& value) {
84-
if (value == kHipoString && !HighsExternalDeps::isAvailable()) {
85-
if (HighsExternalDeps::isAvailableAtCompile()) {
86-
highsLogUser(
87-
report_log_options, HighsLogType::kError,
88-
"The HiPO solver was requested via the \"%s\" option, but this build "
89-
"was compiled without HiPO support. Reconfigure with FAST_BUILD=ON "
90-
"and -DHIPO=ON to enable HiPO.\n",
91-
kSolverString.c_str());
92-
} else {
93-
highsLogUser(
94-
report_log_options, HighsLogType::kError,
95-
"The HiPO solver was requested via the \"%s\" option, but the "
96-
"HiPO dependencies are missing. Install with highspy[hipo] or "
97-
"install "
98-
"highspy-extras. Note, that using HiPO changes the HiGHS license "
99-
"from "
100-
" MIT to Apache, due to the dependencies' licensing.\n",
101-
kSolverString.c_str());
102-
}
103-
return false;
104-
}
10584
if (value == kHighsChooseString || value == kSimplexString ||
10685
value == kIpmString ||
10786
(value == kHipoString && HighsExternalDeps::isAvailable()) ||
10887
value == kIpxString || value == kPdlpString || value == kQpAsmString ||
10988
value == kHiPdlpString)
11089
return true;
111-
highsLogUser(
112-
report_log_options, HighsLogType::kWarning,
113-
"Value \"%s\" for LP solver option (\"%s\") is not one of "
114-
"%s\"%s\", \"%s\", \"%s\", \"%s\" or \"%s\"\n",
115-
value.c_str(), kSolverString.c_str(),
116-
HighsExternalDeps::isAvailable() ? (kHipoString + "\", \"").c_str() : "",
117-
kHighsChooseString.c_str(), kSimplexString.c_str(), kIpmString.c_str(),
118-
kIpxString.c_str(), kPdlpString.c_str(), kQpAsmString.c_str(),
119-
kHiPdlpString.c_str());
120-
return false;
90+
else if (value == kHipoString && !HighsExternalDeps::isAvailable()) {
91+
HighsExternalDeps::logUnavailable(
92+
report_log_options, HighsLogType::kError,
93+
"The HiPO solver was requested via the \"%s\" option.",
94+
kSolverString.c_str());
95+
return false;
96+
} else {
97+
highsLogUser(report_log_options, HighsLogType::kWarning,
98+
"Value \"%s\" for LP solver option (\"%s\") is not one of "
99+
"%s\"%s\", \"%s\", \"%s\", \"%s\" or \"%s\"\n",
100+
value.c_str(), kSolverString.c_str(),
101+
HighsExternalDeps::isAvailable()
102+
? (kHipoString + "\", \"").c_str()
103+
: "",
104+
kHighsChooseString.c_str(), kSimplexString.c_str(),
105+
kIpmString.c_str(), kIpxString.c_str(), kPdlpString.c_str(),
106+
kQpAsmString.c_str(), kHiPdlpString.c_str());
107+
return false;
108+
}
121109
}
122110

123111
bool optionMipLpSolverOk(const HighsLogOptions& report_log_options,
124112
const string& value) {
125-
if (value == kHipoString && !HighsExternalDeps::isAvailable()) {
126-
if (HighsExternalDeps::isAvailableAtCompile()) {
127-
highsLogUser(
128-
report_log_options, HighsLogType::kError,
129-
"The HiPO solver was requested via the \"%s\" option, but this build "
130-
"was compiled without HiPO support. Reconfigure with FAST_BUILD=ON "
131-
"and -DHIPO=ON to enable HiPO.\n",
132-
kMipLpSolverString.c_str());
133-
} else {
134-
highsLogUser(
135-
report_log_options, HighsLogType::kError,
136-
"The HiPO solver was requested via the \"%s\" option, but the "
137-
"HiPO dependencies are missing. Install with highspy[hipo] or "
138-
"install "
139-
"highspy-extras. Note, that using HiPO changes the HiGHS license "
140-
"from "
141-
"MIT to Apache, due to the dependencies' licensing.\n",
142-
kMipLpSolverString.c_str());
143-
}
144-
return false;
145-
}
146-
147113
if (value == kHighsChooseString || value == kSimplexString ||
148114
value == kIpmString ||
149115
(value == kHipoString && HighsExternalDeps::isAvailable()) ||
150116
value == kIpxString)
151117
return true;
152-
highsLogUser(
153-
report_log_options, HighsLogType::kError,
154-
"Value \"%s\" for MIP LP solver option (\"%s\") is not one of "
155-
"%s\"%s\", \"%s\", \"%s\" or \"%s\"\n",
156-
value.c_str(), kMipLpSolverString.c_str(),
157-
HighsExternalDeps::isAvailable() ? (kHipoString + "\", \"").c_str() : "",
158-
kHighsChooseString.c_str(), kSimplexString.c_str(), kIpmString.c_str(),
159-
kIpxString.c_str());
160-
return false;
118+
else if (value == kHipoString && !HighsExternalDeps::isAvailable()) {
119+
HighsExternalDeps::logUnavailable(
120+
report_log_options, HighsLogType::kError,
121+
"The HiPO solver was requested via the \"%s\" option.",
122+
kMipLpSolverString.c_str());
123+
return false;
124+
} else {
125+
highsLogUser(report_log_options, HighsLogType::kError,
126+
"Value \"%s\" for MIP LP solver option (\"%s\") is not one of "
127+
"%s\"%s\", \"%s\", \"%s\" or \"%s\"\n",
128+
value.c_str(), kMipLpSolverString.c_str(),
129+
HighsExternalDeps::isAvailable()
130+
? (kHipoString + "\", \"").c_str()
131+
: "",
132+
kHighsChooseString.c_str(), kSimplexString.c_str(),
133+
kIpmString.c_str(), kIpxString.c_str());
134+
return false;
135+
}
161136
}
162137

163138
bool optionMipIpmSolverOk(const HighsLogOptions& report_log_options,
164139
const string& value) {
165-
if (value == kHipoString && !HighsExternalDeps::isAvailable()) {
166-
if (HighsExternalDeps::isAvailableAtCompile()) {
167-
highsLogUser(
168-
report_log_options, HighsLogType::kError,
169-
"The HiPO solver was requested via the \"%s\" option, but this build "
170-
"was compiled without HiPO support. Reconfigure with FAST_BUILD=ON "
171-
"and -DHIPO=ON to enable HiPO.\n",
172-
kMipIpmSolverString.c_str());
173-
} else {
174-
highsLogUser(
175-
report_log_options, HighsLogType::kError,
176-
"The HiPO solver was requested via the \"%s\" option, but the "
177-
"HiPO dependencies are missing. Install with highspy[hipo] or "
178-
"install "
179-
"highspy-extras. Note, that using HiPO changes the HiGHS license "
180-
"from "
181-
"MIT to Apache, due to the dependencies' licensing.\n",
182-
kMipIpmSolverString.c_str());
183-
}
184-
return false;
185-
}
186140
if (value == kHighsChooseString || value == kIpmString ||
187141
(value == kHipoString && HighsExternalDeps::isAvailable()) ||
188142
value == kIpxString)
189143
return true;
190-
highsLogUser(
191-
report_log_options, HighsLogType::kError,
192-
"Value \"%s\" for MIP IPM solver (\"%s\") option is not one of "
193-
"%s\"%s\", \"%s\" or \"%s\"\n",
194-
value.c_str(), kMipIpmSolverString.c_str(),
195-
HighsExternalDeps::isAvailable() ? (kHipoString + "\", \"").c_str() : "",
196-
kHighsChooseString.c_str(), kIpmString.c_str(), kIpxString.c_str());
197-
return false;
144+
else if (value == kHipoString && !HighsExternalDeps::isAvailable()) {
145+
HighsExternalDeps::logUnavailable(
146+
report_log_options, HighsLogType::kError,
147+
"The HiPO solver was requested via the \"%s\" option.",
148+
kMipIpmSolverString.c_str());
149+
return false;
150+
} else {
151+
highsLogUser(
152+
report_log_options, HighsLogType::kError,
153+
"Value \"%s\" for MIP IPM solver (\"%s\") option is not one of "
154+
"%s\"%s\", \"%s\" or \"%s\"\n",
155+
value.c_str(), kMipIpmSolverString.c_str(),
156+
HighsExternalDeps::isAvailable() ? (kHipoString + "\", \"").c_str()
157+
: "",
158+
kHighsChooseString.c_str(), kIpmString.c_str(), kIpxString.c_str());
159+
return false;
160+
}
198161
}
199162

200163
bool optionHipoParallelTypeOk(const HighsLogOptions& report_log_options,
@@ -389,11 +352,10 @@ OptionStatus checkOptions(const HighsLogOptions& report_log_options,
389352
OptionStatus checkOption(const HighsLogOptions& report_log_options,
390353
const OptionRecordInt& option) {
391354
if (option.lower_bound > option.upper_bound) {
392-
highsLogUser(
393-
report_log_options, HighsLogType::kError,
394-
"checkOption: Option \"%s\" has inconsistent bounds [%" HIGHSINT_FORMAT
395-
", %" HIGHSINT_FORMAT "]\n",
396-
option.name.c_str(), option.lower_bound, option.upper_bound);
355+
highsLogUser(report_log_options, HighsLogType::kError,
356+
"checkOption: Option \"%s\" has inconsistent bounds "
357+
"[%" HIGHSINT_FORMAT ", %" HIGHSINT_FORMAT "]\n",
358+
option.name.c_str(), option.lower_bound, option.upper_bound);
397359
return OptionStatus::kIllegalValue;
398360
}
399361
if (option.default_value < option.lower_bound ||
@@ -614,10 +576,10 @@ OptionStatus setLocalOptionValue(const HighsLogOptions& report_log_options,
614576
bool value_bool;
615577
bool return_status = boolFromString(value_trim, value_bool);
616578
if (!return_status) {
617-
highsLogUser(
618-
report_log_options, HighsLogType::kError,
619-
"setLocalOptionValue: Value \"%s\" cannot be interpreted as a bool\n",
620-
value_trim.c_str());
579+
highsLogUser(report_log_options, HighsLogType::kError,
580+
"setLocalOptionValue: Value \"%s\" cannot be interpreted "
581+
"as a bool\n",
582+
value_trim.c_str());
621583
return OptionStatus::kIllegalValue;
622584
}
623585
return setLocalOptionValue(((OptionRecordBool*)option_records[index])[0],
@@ -1135,13 +1097,13 @@ void reportOption(FILE* file, const HighsLogOptions& log_options,
11351097

11361098
if (!report_only_deviations || option.default_value != *option.value) {
11371099
if (file_type == HighsFileType::kMd) {
1138-
fprintf(
1139-
file,
1140-
"## [%s](@id option-%s)\n- %s\n- Type: string\n- Default: \"%s\"\n\n",
1141-
highsInsertMdEscapes(option.name).c_str(),
1142-
highsInsertMdId(option.name).c_str(),
1143-
highsInsertMdEscapes(option.description).c_str(),
1144-
option.default_value.c_str());
1100+
fprintf(file,
1101+
"## [%s](@id option-%s)\n- %s\n- Type: string\n- Default: "
1102+
"\"%s\"\n\n",
1103+
highsInsertMdEscapes(option.name).c_str(),
1104+
highsInsertMdId(option.name).c_str(),
1105+
highsInsertMdEscapes(option.description).c_str(),
1106+
option.default_value.c_str());
11451107
} else if (file_type == HighsFileType::kFull) {
11461108
fprintf(file, "\n# %s\n", option.description.c_str());
11471109
fprintf(file, "# [type: string, advanced: %s, default: \"%s\"]\n",

0 commit comments

Comments
 (0)