Skip to content

fix: raise clear error when solving model without objective#671

Merged
FabianHofmann merged 4 commits intomasterfrom
fix/issue-668-empty-objective
May 4, 2026
Merged

fix: raise clear error when solving model without objective#671
FabianHofmann merged 4 commits intomasterfrom
fix/issue-668-empty-objective

Conversation

@FBumann
Copy link
Copy Markdown
Collaborator

@FBumann FBumann commented May 4, 2026

Summary

  • Model.solve() now raises ValueError early when no objective has been set, instead of silently writing a malformed LP file with a phantom x variable that later crashes set_int_index.
  • The error message points users to m.add_objective(...), with m.add_objective(0 * x) suggested for the pure-feasibility use case.
  • Updated examples/infeasible-model.ipynb to set m.add_objective(0 * x) before its feasibility-check solve.
  • Added a regression test in test/test_model.py.

Fixes #668

Alternatives considered

  1. Filter null vars rows out of the objective polars df in objective_write_linear_terms. Smallest possible fix — the LP file would no longer contain a phantom x. Rejected because it leaves an empty obj: block whose acceptance varies across solvers/formats, and because silently solving a zero-objective hides what is almost always a user bug.
  2. Write a no-op +0 x0 term when the objective is empty. Avoids the empty-block fragility, but still treats "forgot to set objective" as valid input. Same hide-the-bug objection as (1).
  3. Raise the error (chosen). Makes the omission explicit. Trivial escape hatch (m.add_objective(0 * x)) for users who genuinely want a feasibility solve, which is exactly what the updated example notebook now demonstrates.

Test plan

  • pytest test/test_model.py -k objective passes locally
  • pytest test/test_oetc_settings.py::test_model_solve_forwards_to_oetc passes (test was updated to add an objective)
  • Manual repro from the issue now raises a clear ValueError instead of invalid literal for int() with base 10: 'x'
  • examples/infeasible-model.ipynb executes end-to-end after the fix

🤖 Generated with Claude Code

FBumann and others added 4 commits May 4, 2026 09:10
Previously, `Model.solve()` on a model without an explicit objective
silently wrote a bare ` x` token into the LP file's objective section,
which solvers (e.g. HiGHS) interpreted as a phantom variable named `x`.
Parsing the solution then failed in `set_int_index` with
`ValueError: invalid literal for int() with base 10: 'x'`.

Forgetting to set an objective is almost always a bug, so raise an
informative `ValueError` early in `solve()` instead of silently solving
a zero-objective feasibility problem. Users who genuinely want
feasibility can pass `m.add_objective(0 * x)`.

Fixes #668

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The new "no objective set" guard in `Model.solve()` correctly fires
before the remote-handler branch, so the mock-based forwarding test
needs a real objective to reach the path it is exercising.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The notebook calls `m.solve(...)` purely to trigger infeasibility
diagnostics, but `Model.solve()` now requires an explicit objective.
Add `m.add_objective(0 * x)` with a comment explaining why.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`Model.solve()` now requires an explicit objective; this test was
exercising the cbc unsupported-solver path without one, so add a
trivial objective to reach the assertion under test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@FBumann FBumann requested a review from FabianHofmann May 4, 2026 10:40
@FBumann FBumann marked this pull request as ready for review May 4, 2026 10:40
@FabianHofmann FabianHofmann merged commit bd3450a into master May 4, 2026
22 checks passed
@FabianHofmann FabianHofmann deleted the fix/issue-668-empty-objective branch May 4, 2026 10:54
FBumann added a commit that referenced this pull request May 7, 2026
* docs: restructure upcoming release notes and fold in missing PRs

Group the upcoming version block into Features / Performance / Bug Fixes
/ Breaking Changes / Documentation sections so the headline (piecewise)
leads, and add the entries for #589, #595, #601, #614, #619, #635, #656,
#671, #672, #674. Tighten the piecewise block to its final state.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: tighten upcoming changelog and drop internal-only entries

Trim verbose phrasing in the piecewise / variables / model / solvers
sections, fold subset-superset sub-bullets into one paragraph, and drop
two entries that aren't user-facing for a release notes audience:
sphinx-copybutton (doc tooling) and Model.__weakref__ (only relevant to
extension authors).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: move align convention from breakpoints() to Slopes in changelog

#673 removed the slopes-mode (and slopes_align kwarg) from breakpoints();
the align kwarg now lives on the Slopes class.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: move SOS reformulation bullet from Variables to Model

SOS reformulation is a model-rewrite/solve-pipeline concern, not a
variable attribute.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: split coord alignment into Expressions, move CPLEX to Bug Fixes

- New *Expressions* subsection holds the subset/superset coord
  harmonization, which was misfiled under *Model*.
- CPLEX quality-attribute handling is a fix for crashes on missing
  attributes, not a new feature — moved to **Bug Fixes**.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: fold as_dataarray MultiIndex fix into add_variables bullet

#659 fixes a regression introduced by #614 in the same release cycle —
no end user ever saw the broken state, so a standalone bullet
overstates the change. Net behavior is captured by extending the
add_variables bullet to mention MultiIndex coords.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: tighter pass on upcoming changelog

Drop implementation details that belong in API docs (numpy-vs-pandas
note, JSON encoding for netCDF, "with no auxiliary variables" piecewise
detail), merge the two OETC bullets, and trim "Add X. Supports Y."
wrappers across most lines.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: rephrase active gating bullet to avoid output-zeroing implication

Previous wording ("zeros all auxiliaries when off") was true at the
auxiliary level but glossed over the bounded-tuple case where the
output is not automatically pinned to 0. Drop the implication and
defer the detail to the docstring.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: drop option-name detail from upcoming changelog

Trim references to specific kwargs/attributes the reader doesn't need
in the high-level summary: method="auto" parens, align="pieces|leading",
deep / include_solution, reformulate_sos="auto", solver_name /
**solver_options, max_dual_infeasibility example, and the
operator-by-operator coord-alignment breakdown.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Model.solve() crashes with ValueError when no objective is set — phantom x variable in LP file

2 participants