Skip to content

audit: .nl format decoding is implemented twice (discopt nl_parser.rs and pounce-nl) — decide whether to factor #1213

Description

@jkitchin

The finding

discopt depends on the pounce-solver wheel, which ships pounce-nl"AMPL .nl reader, reverse-mode AD tape, and TNLP evaluator" — exposing read_nl, parse_nl_text and build_nl_problem at the Python top level. discopt separately maintains crates/discopt-core/src/nl_parser.rs (3,527 lines) for the same file format.

Two implementations of the same format spec live in the same dependency closure, in two projects owned by the same author. Format-level bugs, dialect quirks and opcode coverage have to be fixed twice.

Important qualification — they are NOT drop-in duplicates

This was my first reading and it is wrong; recording the correction so nobody acts on it.

produces usable for
pounce.read_nl NlProblemobjective, gradient, constraints, jacobian(_structure), hessian(_structure), x_l/x_u, x0, names evaluation (a TNLP)
discopt parse_nl_file ModelRepr — the expression arena structure: McCormick envelopes, FBBT, term classification, convexity detection, presolve

You cannot build a relaxation from an evaluable TNLP. There is no NlProblem → ModelRepr lowering and there cannot be one — the structure is gone by then. So replacing nl_parser.rs with pounce-nl is not an option, and any plan that assumes it is should stop here.

discopt's parser additionally handles two things NlProblem does not surface:

  • complementarity (type-5) rowsparse_nl_with_complementarity recovers pairs from the r segment (nl_parser.rs:738-750), consumed by the MPEC path;
  • suffixes (nl_parser.rs:1800).

What is actually shared

Only the bottom layer: the .nl decoder — header parsing, segment dispatch, opcode decoding, the .col/.row sidecar convention. Above that, one side builds an arena and the other an evaluable tape. That is a legitimate divergence, not redundancy.

So the question this issue exists to answer is narrow: is the shared decoding layer large enough, and stable enough, to be worth factoring into one crate with two builders on top?

Why this is an audit, not a port

The payoff is maintenance-only — no performance, no capability. Against that:

  • nl_parser.rs is battle-tested against the ~4,800-instance MINLPLib snapshot and the 66-instance in-repo corpus. Churning it risks the corpus for no user-visible gain.
  • Cross-crate edges have a measured cost in this repo. Cargo.toml records feral 0.11.2 → 0.11.3 silently regressing set-covering from ~3 s to a >30 s timeout on a patch bump (an LU refactor going quadratic→cubic in row count), now guarded by test_setcover_lp_regression.py. Every new edge also inherits the bound-neutrality bump protocol (the 49/49 bit-identical panel).

A plausible and acceptable outcome of this issue is "keep both, document why" — which is still worth having written down, because the overlap will otherwise be rediscovered.

Tasks

  • Measure the actual shared surface: how much of nl_parser.rs is format decoding vs ModelRepr construction? (A line-level split, not an estimate.)
  • Compare opcode/segment coverage against pounce-nl — does either handle cases the other refuses? Complementarity and suffixes are known discopt-only; check the reverse direction too (AMPLFUNC external-function resolution is a known pounce-nl feature).
  • Decide and record: factor a shared decoder crate, or keep both with the rationale documented in docs/dev/.
  • If factoring: the corpus is the gate — every one of the 66 in-repo instances must parse to a byte-identical ModelRepr, and complementarity/suffix handling must be preserved, before anything is deleted.

Related

  • kernel: standalone discopt-solve binary (.nl → certificate, zero Python) + apples-to-apples SCIP comparison #1212 (standalone discopt-solve binary + SCIP comparison) — that issue should not block on this one. It needs ModelRepr, which only discopt's parser produces today. Worth noting there that pounce-cli already solves .nl files, so the two CLIs are a second, separate overlap to think about.
  • Broader context: this came out of asking whether discopt's Rust components (notably lp/simplex/, 11,384 lines of warm-started dual simplex, which POUNCE lacks) belong in the POUNCE crate family. That is a larger question and is deliberately not in scope here.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions