From 15955de2044c8a66f95d0f1fb25424bda9b4bc81 Mon Sep 17 00:00:00 2001 From: Zedong Peng Date: Thu, 20 Aug 2026 13:59:09 -0400 Subject: [PATCH] fix: MPS reader integer default bounds --- src/mps_parser.c | 151 ++++++++++++++++++++++++++++++++++++------ test/test_read_mps.py | 114 +++++++++++++++++++++++++++++++ 2 files changed, 246 insertions(+), 19 deletions(-) diff --git a/src/mps_parser.c b/src/mps_parser.c index 5ae437b..e98fb7a 100644 --- a/src/mps_parser.c +++ b/src/mps_parser.c @@ -26,6 +26,7 @@ limitations under the License. #include #define READER_BUFFER_SIZE (4 * 1024 * 1024) +#define MPS_INFINITE_BOUND 1e20 typedef struct NameNode { @@ -319,6 +320,9 @@ typedef struct double *var_upper_bounds; double *constraint_lower_bounds; double *constraint_upper_bounds; + unsigned char *col_binary_default; + unsigned char *col_has_lower; + int in_integer_block; size_t col_capacity; size_t constraint_capacity; @@ -365,12 +369,17 @@ static bool ensure_column_capacity(MpsParserState *state) state->objective_coeffs = (double *)safe_realloc(state->objective_coeffs, new_cap * sizeof(double)); state->var_lower_bounds = (double *)safe_realloc(state->var_lower_bounds, new_cap * sizeof(double)); state->var_upper_bounds = (double *)safe_realloc(state->var_upper_bounds, new_cap * sizeof(double)); + state->col_binary_default = + (unsigned char *)safe_realloc(state->col_binary_default, new_cap * sizeof(unsigned char)); + state->col_has_lower = (unsigned char *)safe_realloc(state->col_has_lower, new_cap * sizeof(unsigned char)); for (size_t i = state->col_capacity; i < new_cap; ++i) { state->objective_coeffs[i] = 0.0; state->var_lower_bounds[i] = 0.0; state->var_upper_bounds[i] = INFINITY; + state->col_binary_default[i] = 0; + state->col_has_lower[i] = 0; } state->col_capacity = new_cap; @@ -395,9 +404,46 @@ typedef enum SEC_RANGES, SEC_BOUNDS, SEC_OBJSENSE, + SEC_SOS, + SEC_UNSUPPORTED, SEC_ENDATA } MpsSection; +/* Sections that change the model beyond an LP: refuse the file rather than + * silently dropping them (HiGHS does the same for the ones it cannot parse). */ +static const char *const MPS_UNSUPPORTED_SECTIONS[] = {"QUADOBJ", + "QMATRIX", + "QSECTION", + "QCMATRIX", + "CSECTION", + "INDICATORS", + "DELAYEDROWS", + "MODELCUTS", + "USERCUTS", + "GENCONS", + "PWLOBJ", + "PWLNAM", + "PWLCON"}; + +/* Bounds at or beyond +/-1e20 are the conventional way of writing infinity in + * MPS files; HiGHS applies the same threshold (options.infinite_bound). */ +static int apply_infinite_bounds(double *lower, double *upper, size_t n, const char *what) +{ + for (size_t i = 0; i < n; ++i) + { + if (lower[i] >= MPS_INFINITE_BOUND || upper[i] <= -MPS_INFINITE_BOUND) + { + fprintf(stderr, "ERROR: %s %zu has bounds [%g, %g].\n", what, i, lower[i], upper[i]); + return -1; + } + if (lower[i] <= -MPS_INFINITE_BOUND) + lower[i] = -INFINITY; + if (upper[i] >= MPS_INFINITE_BOUND) + upper[i] = INFINITY; + } + return 0; +} + lp_problem_t *read_mps_file(const char *filename) { MpsParserState state = {0}; @@ -437,22 +483,42 @@ lp_problem_t *read_mps_file(const char *filename) if (isalpha((unsigned char)tokens[0][0])) { + /* A section header is alone on its line; OBJSENSE may carry its value. + * Checking the token count first keeps this off the hot path. */ MpsSection next_section = SEC_NONE; - if (strcmp(tokens[0], "ROWS") == 0) - next_section = SEC_ROWS; - else if (strcmp(tokens[0], "COLUMNS") == 0) - next_section = SEC_COLUMNS; - else if (strcmp(tokens[0], "RHS") == 0) - next_section = SEC_RHS; - else if (strcmp(tokens[0], "RANGES") == 0) - next_section = SEC_RANGES; - else if (strcmp(tokens[0], "BOUNDS") == 0) - next_section = SEC_BOUNDS; + if (n_tokens == 1) + { + if (strcmp(tokens[0], "ROWS") == 0) + next_section = SEC_ROWS; + else if (strcmp(tokens[0], "COLUMNS") == 0) + next_section = SEC_COLUMNS; + else if (strcmp(tokens[0], "RHS") == 0) + next_section = SEC_RHS; + else if (strcmp(tokens[0], "RANGES") == 0) + next_section = SEC_RANGES; + else if (strcmp(tokens[0], "BOUNDS") == 0) + next_section = SEC_BOUNDS; + else if (strcmp(tokens[0], "OBJSENSE") == 0 || strcmp(tokens[0], "OBJSENS") == 0) + next_section = SEC_OBJSENSE; + else if (strcmp(tokens[0], "SOS") == 0 || strcmp(tokens[0], "SETS") == 0) + next_section = SEC_SOS; + else if (strcmp(tokens[0], "ENDATA") == 0) + next_section = SEC_ENDATA; + else + { + for (size_t k = 0; k < sizeof(MPS_UNSUPPORTED_SECTIONS) / sizeof(MPS_UNSUPPORTED_SECTIONS[0]); ++k) + { + if (strcmp(tokens[0], MPS_UNSUPPORTED_SECTIONS[k]) == 0) + { + next_section = SEC_UNSUPPORTED; + break; + } + } + } + } else if (strcmp(tokens[0], "OBJSENSE") == 0 || strcmp(tokens[0], "OBJSENS") == 0) - next_section = SEC_OBJSENSE; - else if (strcmp(tokens[0], "ENDATA") == 0) { - next_section = SEC_ENDATA; + next_section = SEC_OBJSENSE; } bool inline_max = next_section == SEC_OBJSENSE && n_tokens >= 2 && @@ -463,6 +529,12 @@ lp_problem_t *read_mps_file(const char *filename) if (is_header) { + if (next_section == SEC_UNSUPPORTED) + { + fprintf(stderr, "ERROR: MPS file reader cannot parse %s section.\n", tokens[0]); + state.error_flag = 1; + break; + } if (current_section == SEC_ROWS && next_section != SEC_ROWS && !rows_finalized) { if (finalize_rows(&state) != 0) @@ -515,6 +587,8 @@ lp_problem_t *read_mps_file(const char *filename) if (parse_bounds_section(&state, tokens, n_tokens) != 0) state.error_flag = 1; break; + case SEC_SOS: + break; default: break; @@ -530,6 +604,23 @@ lp_problem_t *read_mps_file(const char *filename) return NULL; } + for (size_t i = 0; i < state.col_map.size; ++i) + { + if (state.col_binary_default[i]) + { + state.var_lower_bounds[i] = 0.0; + state.var_upper_bounds[i] = 1.0; + } + } + + if (apply_infinite_bounds(state.var_lower_bounds, state.var_upper_bounds, state.col_map.size, "Column") != 0 || + apply_infinite_bounds( + state.constraint_lower_bounds, state.constraint_upper_bounds, state.row_map.size, "Row") != 0) + { + free_parser_state(&state); + return NULL; + } + lp_problem_t *prob = safe_calloc(1, sizeof(lp_problem_t)); prob->num_variables = state.col_map.size; @@ -600,10 +691,8 @@ static int finalize_rows(MpsParserState *state) } } - if (obj_idx == -1 && state->num_buffered_rows > 0) - { - obj_idx = 0; - } + if (obj_idx == -1) + fprintf(stderr, "WARNING: No objective (N) row found in ROWS section; objective is zero.\n"); if (obj_idx != -1) { @@ -667,6 +756,13 @@ static int parse_columns_section(MpsParserState *state, char **tokens, int n_tok if (n_tokens >= 2 && strcmp(tokens[1], "'MARKER'") == 0) { + if (n_tokens >= 3) + { + if (strcmp(tokens[2], "'INTORG'") == 0) + state->in_integer_block = 1; + else if (strcmp(tokens[2], "'INTEND'") == 0) + state->in_integer_block = 0; + } return 0; } @@ -697,9 +793,12 @@ static int parse_columns_section(MpsParserState *state, char **tokens, int n_tok if (!ensure_column_capacity(state)) return -1; + size_t n_cols_before = state->col_map.size; int col_idx = namemap_put(&state->col_map, col_name); if (col_idx == -1) return -1; + if (state->col_map.size > n_cols_before && state->in_integer_block) + state->col_binary_default[col_idx] = 1; for (int i = pair_start_index; i + 1 < n_tokens; i += 2) { @@ -811,27 +910,38 @@ static int parse_bounds_section(MpsParserState *state, char **tokens, int n_toke if (col_idx == -1) return 0; - if (strcmp(bound_type, "LO") == 0) + /* Any BOUNDS entry cancels the implicit [0, 1] default of an integer column. */ + state->col_binary_default[col_idx] = 0; + + if (strcmp(bound_type, "LO") == 0 || strcmp(bound_type, "LI") == 0) { state->var_lower_bounds[col_idx] = value; + state->col_has_lower[col_idx] = 1; } - else if (strcmp(bound_type, "UP") == 0) + else if (strcmp(bound_type, "UP") == 0 || strcmp(bound_type, "UI") == 0) { state->var_upper_bounds[col_idx] = value; + /* A negative upper bound on a column with no explicit lower bound means + * the lower bound is -infinity (CPLEX/Gurobi/SCIP convention). */ + if (value < 0.0 && !state->col_has_lower[col_idx]) + state->var_lower_bounds[col_idx] = -INFINITY; } else if (strcmp(bound_type, "FX") == 0) { state->var_lower_bounds[col_idx] = value; state->var_upper_bounds[col_idx] = value; + state->col_has_lower[col_idx] = 1; } else if (strcmp(bound_type, "FR") == 0) { state->var_lower_bounds[col_idx] = -INFINITY; state->var_upper_bounds[col_idx] = INFINITY; + state->col_has_lower[col_idx] = 1; } else if (strcmp(bound_type, "MI") == 0) { state->var_lower_bounds[col_idx] = -INFINITY; + state->col_has_lower[col_idx] = 1; } else if (strcmp(bound_type, "PL") == 0) { @@ -841,6 +951,7 @@ static int parse_bounds_section(MpsParserState *state, char **tokens, int n_toke { state->var_lower_bounds[col_idx] = 0.0; state->var_upper_bounds[col_idx] = 1.0; + state->col_has_lower[col_idx] = 1; } return 0; } @@ -905,6 +1016,8 @@ static void free_parser_state(MpsParserState *state) free(state->objective_coeffs); free(state->var_lower_bounds); free(state->var_upper_bounds); + free(state->col_binary_default); + free(state->col_has_lower); free(state->constraint_lower_bounds); free(state->constraint_upper_bounds); free(state->objective_row_name); diff --git a/test/test_read_mps.py b/test/test_read_mps.py index 6167fa6..827c6f5 100644 --- a/test/test_read_mps.py +++ b/test/test_read_mps.py @@ -162,3 +162,117 @@ def test_read_and_optimize_maximize(mps_max_file, atol): assert model.Status == PDLP.OPTIMAL, f"Unexpected termination status: {model.Status}" assert np.allclose(model.X, [1.5, 1.75], atol=atol), f"Unexpected primal solution: {model.X}" assert np.isclose(model.ObjVal, 3.25, atol=atol), f"Unexpected objective value: {model.ObjVal}" + + +# --------------------------------------------------------------------------- +# BOUNDS-section semantics +# --------------------------------------------------------------------------- + +MPS_BOUNDS = """NAME BOUNDS +ROWS + N COST + L R1 +COLUMNS + C0 COST 1.0 R1 1.0 + MARKER 'MARKER' 'INTORG' + I1 COST 1.0 R1 1.0 + I2 COST 1.0 R1 1.0 + I3 COST 1.0 R1 1.0 + I4 COST 1.0 R1 1.0 + I5 COST 1.0 R1 1.0 + MARKER 'MARKER' 'INTEND' + C6 COST 1.0 R1 1.0 + C7 COST 1.0 R1 1.0 + C8 COST 1.0 R1 1.0 +RHS + RHS R1 5.0 +BOUNDS + UP BND I2 5.0 + LO BND I3 2.0 + MI BND I4 + UP BND I5 -3.0 + UP BND C6 -3.0 + LO BND C7 -10.0 + UP BND C7 -3.0 + LO BND C8 -1e30 + UP BND C8 1e20 +ENDATA +""" + + +def _read_text(tmp_path, name, text): + path = tmp_path / name + path.write_text(text) + return cupdlpx.read(path) + + +def test_read_bounds_conventions(tmp_path): + """ + - integer (MARKER) columns with no BOUNDS entry default to [0, 1] + - any BOUNDS entry on such a column cancels that default + - negative UP with no explicit lower bound implies lb = -inf, but an + explicit lower bound (C7) is kept + - |bound| >= 1e20 is treated as infinite + """ + model = _read_text(tmp_path, "bounds.mps", MPS_BOUNDS) + inf = np.inf + # C0 I1 I2 I3 I4 I5 C6 C7 C8 + expected_lb = [0.0, 0.0, 0.0, 2.0, -inf, -inf, -inf, -10.0, -inf] + expected_ub = [inf, 1.0, 5.0, inf, inf, -3.0, -3.0, -3.0, inf] + assert np.array_equal(model.lb, expected_lb), model.lb + assert np.array_equal(model.ub, expected_ub), model.ub + + +MPS_NO_OBJ_ROW = """NAME NOOBJ +ROWS + E R1 + L R2 +COLUMNS + X1 R1 1.0 R2 1.0 + X2 R1 2.0 +RHS + RHS R1 5.0 R2 2.0 +ENDATA +""" + + +def test_read_without_objective_row(tmp_path): + """ + A file with no N row has a zero objective; all rows stay constraints + (previously the first row was silently taken as the objective). + """ + model = _read_text(tmp_path, "noobj.mps", MPS_NO_OBJ_ROW) + assert model.num_vars == 2 + assert model.num_constrs == 2 + assert np.allclose(model.c, [0.0, 0.0]) + assert np.allclose(model.constr_lb, [5.0, -np.inf]) + assert np.allclose(model.constr_ub, [5.0, 2.0]) + + +MPS_SOS = MPS_MIN.replace( + "ENDATA\n", + "SOS\n S1 SOS s1\n X1 1\n X2 2\nENDATA\n", +) + +MPS_QUADOBJ = MPS_MIN.replace( + "ENDATA\n", + "QUADOBJ\n X1 X1 2.0\nENDATA\n", +) + + +def test_read_sos_section_is_ignored(tmp_path): + """ + SOS only restricts the integer feasible set, so the LP relaxation is + unchanged and the section is skipped. + """ + model = _read_text(tmp_path, "sos.mps", MPS_SOS) + _check_min_model_data(model) + + +def test_read_unsupported_section_raises(tmp_path): + """ + Sections that change the model beyond an LP (quadratic terms, cones, + indicators, ...) must not be silently dropped. + """ + with pytest.raises(Exception): + _read_text(tmp_path, "quadobj.mps", MPS_QUADOBJ)