Skip to content

Add LP format reader; accept .lp wherever .mps is accepted#1120

Draft
mlubin wants to merge 9 commits intoNVIDIA:mainfrom
mlubin:lp-format
Draft

Add LP format reader; accept .lp wherever .mps is accepted#1120
mlubin wants to merge 9 commits intoNVIDIA:mainfrom
mlubin:lp-format

Conversation

@mlubin
Copy link
Copy Markdown
Contributor

@mlubin mlubin commented Apr 17, 2026

Summary

  • New LP format parser (cuopt::mps_parser::parse_lp) in libmps_parser, supporting LP, MIP, and QP. Unsupported LP sections (SOS, PWL,
    semi-continuous, user cuts, general constraints) are rejected with a clear ValidationError.
  • Extension-based dispatch via parse_optimization_file(): .lp → LP parser, anything else → MPS. Plumbed through cuopt_cli, cuOptReadProblem,
    and the self-hosted client.
  • Python ParseLp next to ParseMps, sharing the existing data-model marshaling path.
  • No server-side change — the self-hosted client parses locally and sends the data model as JSON/msgpack/zlib to the server.
  • Shared finalize_problem helper extracted from the MPS parser so both parsers populate mps_data_model_t through one code path.
  • Docs updated; neutral "LP format" wording with the specific dialect cited by URL where readers need the authoritative reference.

Test plan

  • libmps_parser unit tests — LP_PARSER_TEST 33/33 (28 LP + 5 dispatch), MPS_PARSER_TEST 41/41 (behavior preserved through the refactor).
  • C API test — new c_api.read_lp_file_by_extension gtest passes.
  • Python — pytest python/cuopt/cuopt/tests/linear_programming/test_parser.py 5/5 (including 3 new ParseLp tests).
  • cuopt_cli runs on both .lp and .mps inputs
  • CI green on this branch.

Checklist

  • I am familiar with the Contributing Guidelines.

  • Testing

    • New or existing tests cover these changes
    • Added tests
    • Created an issue to follow-up
    • NA
  • Documentation

    • The documentation is up to date with these changes
    • Added new documentation
    • NA

    🤖 Generated with Claude Code

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Apr 17, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@mlubin
Copy link
Copy Markdown
Contributor Author

mlubin commented Apr 18, 2026

/ok to test 870a37f

@rg20 rg20 added feature request New feature or request non-breaking Introduces a non-breaking change labels Apr 20, 2026
@rg20 rg20 added this to the 26.06 milestone Apr 20, 2026
if (lower == "such" && name_equals_ci(t2, "that")) return true;
if (lower == "st" || lower == "s.t.") return true;
if (lower == "lazy" && name_equals_ci(t2, "constraints")) return true;
if (lower == "user" && name_equals_ci(t2, "cuts")) return true;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not support "user cuts"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're not supported; added a test to confirm. Recognizing the section here allows us to print a more helpful error message, I believe.

if (lower == "semi" && peek(1).kind == LpTokenKind::Minus &&
name_equals_ci(peek(2), "continuous"))
return true;
if (lower == "sc") return true;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably should support semicontinuous variables

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's easy to add, though I'd rather let #1096 land first to avoid conflicts.

@mlubin
Copy link
Copy Markdown
Contributor Author

mlubin commented Apr 20, 2026

/ok to test cf03235

@mlubin
Copy link
Copy Markdown
Contributor Author

mlubin commented Apr 20, 2026

/ok to test 003ecff

mlubin and others added 9 commits April 21, 2026 21:13
Move the mps_parser_t::fill_problem() body into a free template
finalize_problem() in a new header parser_finalize.hpp. The function
reads its inputs via duck typing and uses requires-expressions to
handle the MPS-specific fields (ranges_values, qmatrix_entries,
objective_scaling_factor_value) so the same helper will work for any
parser that exposes the same field shape — a prerequisite for adding
an LP parser that shares finalization logic.

Pure refactor: mps_parser behavior, the data it populates, and all 41
mps_parser unit tests are unchanged.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Miles Lubin <mlubin@nvidia.com>
Add parse_lp<i_t, f_t>(path) to libmps_parser. The parser reads an
optimization problem in LP format (LP, MIP, and QP supported) and
returns an mps_data_model_t, so LP inputs are fully interchangeable
with MPS at the data-model level.

Implementation is a hand-written tokenizer + section-driven parser
that lives entirely in src/lp_parser.cpp (public surface is just the
free function). Out-of-scope LP sections (SOS, PWL objective,
semi-continuous variables, user cuts, general constraints) are
detected and rejected with a ValidationError.

Also add parse_optimization_file<i_t, f_t>(path) as a shared
entry point for tools that want to auto-dispatch on file extension:
a case-insensitive ".lp" suffix routes to parse_lp, everything else
(including .mps, .mps.gz, .mps.bz2, and extensionless paths) to
parse_mps.

Tests: 33 gtest cases (28 LP parser + 5 dispatch) covering minimize/
maximize, equality/inequality/mixed constraints, free + negative-lb
bounds, generals/binaries/mixed MIP, QP with diagonal and cross
terms, MIQP, auto-named anonymous constraints, ranges/infinity in
bounds, quadratic with and without "/2", and error paths for
unsupported sections and missing objective.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Miles Lubin <mlubin@nvidia.com>
Thread LP support through every user-facing entry point that currently
takes an MPS file. Each surface dispatches on the file extension —
".lp" routes to the LP parser, anything else to MPS — so existing .mps
users are unaffected and .lp inputs "just work".

Surfaces updated:
- cuopt_cli: use parse_optimization_file() instead of parse_mps()
  directly.
- C API (cuOptReadProblem): dispatch via parse_optimization_file()
  and recognize both MPS and LP error messages in the file-open path.
- Cython bridge: add call_parse_lp() alongside call_parse_mps() and
  expose it to Python.
- Python bindings: add ParseLp(lp_file_path) next to ParseMps() and
  factor the data-model marshaling into a shared cdef helper so both
  Python entry points share one copy of the ~100-line marshaling
  path.
- Self-hosted client: _parse_file_to_data_model (renamed from
  _mps_parse) dispatches on .lp vs .mps. The client parses locally
  and ships the data model as JSON/msgpack/zlib to the server, so
  no server change is needed — LP is supported transparently.

Tests: new c_api.read_lp_file_by_extension gtest confirms .lp
dispatch through the C API. Three new Python tests exercise
ParseLp on a minimal LP, verify rejection of unsupported sections,
and verify that ParseLp / ParseMps produce equivalent data models
for the same problem.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Miles Lubin <mlubin@nvidia.com>
Update user-facing documentation so LP format inputs are as
discoverable as MPS:

- cuopt-cli: index and cli-examples call out the .lp / .mps
  extension dispatch rule and the LP-format dialect link.
- C API (lp-qp-example.rst): cross-ref the new LP section from
  the MPS example and add an LP-file section next to it.
- New lp_file_example.c + sample.lp mirroring the MPS example;
  cuOptReadProblem handles both without code changes.
- Server lp-examples.rst: section title and intro mention LP.
- hidden/mps-api.rst: expose ParseLp autofunction next to
  ParseMps.

Prose uses neutral "LP format" wording; the specific LP dialect
implemented is cited by URL where users need the authoritative
reference.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Miles Lubin <mlubin@nvidia.com>
Signed-off-by: Miles Lubin <mlubin@nvidia.com>
Merge lp_parser_test.cpp into parser_test.cpp (renamed from
mps_parser_test.cpp) and restructure the paired-format tests around
gtest fixtures:

- Introduce per-fixture TEST_F classes (good_mps_1_test,
  up_low_bounds_test, fixed_var_bound_test, mip_with_bounds_test, etc.),
  each owning a single check_model() helper that encodes the expected
  parsed data model for one named problem. The MPS and LP TEST_F cases
  inside each fixture parse their respective input file and call the
  same check_model — the expected values live in exactly one place per
  fixture, and readers see the MPS/LP pairing at a glance.
- Add 10 LP fixture files (datasets/linear_programming/*.lp and
  datasets/mixed_integer_programming/*.lp) alongside the existing .mps
  fixtures they mirror semantically.
- Pull in all LP-specific coverage from the old lp_parser_test.cpp
  (constraint relations =/>=, QP diagonal/cross terms, MIQP, error
  paths for unsupported SOS/Semi-continuous/PWLObj sections,
  case-insensitive keywords, backslash comments, auto-generated
  constraint names, parse_optimization_file extension dispatch, etc.).
- Drop the LP_PARSER_TEST target from CMakeLists.txt; everything now
  runs under a single PARSER_TEST binary (87 tests in 20 suites).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Miles Lubin <mlubin@nvidia.com>
…c rules

Five changes to the LP parser, each with a corresponding test:

- Accept 'st.' (with a trailing period) as a Subject-To section synonym
  alongside the existing 'st' and 's.t.' forms.

- Accept '=<' and '=>' as swapped-form relational operators, equivalent
  to '<=' and '>=' respectively, in both constraints and bounds.

- Expand variable-name character classes to match the LP-format
  convention. Starts: letters plus `! " # $ % & ( ) , ; ? @ _ \` ' { } | ~`;
  continuation characters add digits, `.`, and `/`. Characters used by
  the grammar (`+ - * ^ : = < > [ ] \` whitespace) remain excluded.

- Reject a negative upper bound with no explicit lower bound. Previously
  `x <= -1` alone would silently produce an infeasible 0 ≤ x ≤ -1 by
  colliding with the default lower of 0. The parser now flags this at
  parse time and suggests `-inf <= x <= -1` or pairing an explicit lower
  bound alongside. A new `lower_explicitly_set_` set tracks which
  variables got a lower bound via '>=', '=', 'free', or the 'lb <= x'
  form; a post-pass over the Bounds section checks each variable with a
  negative upper.

- Require the '/ 2' suffix after a quadratic objective bracket. Without
  it there is no ambiguity-free way to tell whether the user meant '/ 2'
  and forgot or intended the bare coefficients; the earlier permissive
  behavior is replaced with a parse error.

PARSER_TEST now runs 93 tests (up from 87); all pass. One existing test
(`quadratic_without_slash_two_divides_coefficients_in_place`) is
replaced with `quadratic_without_slash_two_is_rejected` to reflect the
new stricter behavior. New tests: `subject_to_variant_st_dot`,
`swapped_relational_operators_eq_lt_and_eq_gt`,
`variable_names_with_special_characters`,
`negative_upper_without_explicit_lower_throws`,
`negative_upper_with_explicit_lower_ok`,
`negative_upper_with_range_bound_ok`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Miles Lubin <mlubin@nvidia.com>
Previously 'Lazy Constraints' was accepted and parsed as a regular
constraints block. This quietly pulled in content that's meant to be
solver-side metadata, not model structure. Treat it the same as 'User
Cuts': recognize the header as a section boundary so the prior section
ends cleanly, then throw with a clear "not supported (scope is
LP/MIP/QP only)" message.

Removes the LazyConstraints enumerator and its dispatch arm, adds the
display-name branch so reject_unsupported_section() prints "Lazy
Constraints", and adds two tests (unsupported_lazy_constraints_section_throws
and unsupported_user_cuts_section_throws). PARSER_TEST: 95/95 pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Miles Lubin <mlubin@nvidia.com>
Replace the four mentions of Gurobi and their docs URL with a neutral
description: the parser accepts the conventional LP dialect implemented
by most commercial optimization solvers (not the lpsolve variant, which
has a different syntax). Files touched:

- cpp/libmps_parser/include/mps_parser/lp_parser.hpp (public-header
  doxygen)
- python/cuopt/cuopt/linear_programming/cuopt_mps_parser/parser.py
  (ParseLp docstring)
- docs/cuopt/source/cuopt-c/lp-qp-milp/lp-qp-example.rst
- docs/cuopt/source/cuopt-cli/cli-examples.rst

Comment-only change; no behavior impact. PARSER_TEST: 95/95 pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Miles Lubin <mlubin@nvidia.com>
// Purely linear term inside the brackets — permitted as long as the
// surrounding /2 convention is respected (the linear term is scaled
// the same way as the quadratic ones).
out_linear.push_back({i1, sign * coeff});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was skimming this to see if there was anything of interest for JuMP's reader. We don't support linear terms inside [].

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sounds like there's no reason for us to allow this.

// Require the "/ 2" suffix after a quadratic objective expression.
// Without it there is no ambiguity-free way to tell whether the user
// meant /2 and forgot vs. intended bare coefficients, so we enforce the
// stricter form.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple solvers treat the ]/2 as optional in the objective. I don't remember if I just copied them, or if there were some common instances where this happened.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed multiple solvers treat ]/2 as optional in the objective, but it seems scary to me. CPLEX says it's required, IIRC. I'd rather disallow this unless there's a good reason otherwise.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CPLEX says it's required

Oh, they all say it's required. But the files that they can actually read depart quite a lot from the written spec.


// A negative upper bound requires an explicitly stated lower bound,
// otherwise the default lower of 0 would collide with the upper and make
// the variable silently infeasible. Flag this at parse time.
Copy link
Copy Markdown

@odow odow Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I think if the upper bound is negative, we assume that the lower bound is -Inf. But I see now that Gurobi flags the model as infeasible.

x-ref jump-dev/MathOptInterface.jl#2994

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature request New feature or request non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants