Skip to content

Add lower_bound()/upper_bound() expression operators - #264

Open
Juliette-Gerbaux wants to merge 131 commits into
mainfrom
feat/lb_ub_operator
Open

Add lower_bound()/upper_bound() expression operators#264
Juliette-Gerbaux wants to merge 131 commits into
mainfrom
feat/lb_ub_operator

Conversation

@Juliette-Gerbaux

@Juliette-Gerbaux Juliette-Gerbaux commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Process ID

Process: GP-01

Description

Adds two new expression-language operators, lower_bound(variable_name) and upper_bound(variable_name), that return a variable's current (post-solve) lower/upper bound. This lets results surface bound mutations applied by thermal heuristics (see the integer-strategy/thermal-heuristic work already on this branch), which previously had no way to be inspected in outputs.

The new LowerBoundNode/UpperBoundNode are wired through every ExpressionVisitor implementation (copy, degree, equality, indexing, print, evaluate, uses_sum_connections_on) and through the port-field/model validators. They resolve at parse time to a bare model-variable identifier, and are restricted to extra-outputs and port-field-definitions — using them in constraints, binding-constraints, objective contributions, or variable-bound expressions raises ValueError.

On the runner side, OptimizationProblem.get_variable_solution is refactored into a shared _reassemble_variable_attr(model_id, var_name, attr) helper, parameterized over linopy.Variable's solution/lower/upper attributes, so the existing relaxed/exact merged-variable-group correctness fix now also applies to bound retrieval. Two new accessors, get_variable_lower_bound/get_variable_upper_bound, are added and plumbed through SimulationTableBuilder and VectorizedExtraOutputBuilder so extra-outputs can evaluate the new operators.

Impact Analysis

  • Modules affected: expression/ (2 new node types + all visitor implementations), simulation/ (optimization.py, extra_output.py, simulation_table.py, vectorized_builder.py).
  • Not touched: study/, optim_config/.
  • Solver output values: not expected to change. This PR is purely additive — new read-only expression nodes and accessors, no change to constraint/objective construction. The get_variable_solution refactor preserves the exact same reassembly logic for the solution attribute (same per-component lookup + xr.concat), just generalized to also read lower/upper; existing solved outputs are unaffected.

Checklist

  • Unit tests pass (pytest)
  • Type checking passes (mypy)
  • Formatting passes (black, isort)
  • pyproject.toml version bumped if applicable
  • AGENTS.md reviewed for impact and updated if needed

Juliette-Gerbaux and others added 22 commits July 13, 2026 15:10
Cross-check each heuristic input/output declared in optim-config.yml against
the model: the referenced id must exist and have the time-dependence the
fast/accurate thermal heuristics expect (e.g. min_up_duration constant,
generation_power per-timestep), catching mismatches at load time instead of
a runtime crash mid-solve. nb_units_max and cluster_max_generation now
accept either form, with the heuristics broadcasting a scalar internally.
Also warn (instead of silently truncating) when min_up_duration/
min_down_duration resolve to a non-integer number of timesteps.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The merged Variable rebuilt for split integer/binary variables was a
detached xr.concat copy: setting .lower/.upper on it silently wrote to
an orphaned copy instead of the solver, and its name/label_range were
inherited from only one of the two groups. Now fails loudly and
carries correct metadata.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Documents the new per-component integer relaxation strategy
(exact/relaxed/heuristic) and the built-in fast/accurate thermal
heuristics in optim-config.md, building.md, AGENTS.md, and the
changelog. Also fixes a mypy error-code annotation in optimization.py.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Surfaces a variable's current bound in extra-outputs and port-field
definitions, mirroring dual()/reduced_cost(). This lets heuristic-mutated
bounds (e.g. the fast thermal heuristic tightening generation_power's
lower bound) be read back post-solve instead of only the solved value.
Reading bypasses the detached merged relaxed/exact variable copy the same
way get_variable_solution() already does, so mutations reach the output.

Use the new operator to fix num_units_on/non_prop_cost in the thermal
heuristic model libraries: they previously approximated unit commitment
from generation_power / max_power_per_unit, which diverges from what the
fast heuristic actually enforces; deriving it from
lower_bound(generation_power) / min_power_per_unit matches it exactly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Juliette-Gerbaux
Juliette-Gerbaux requested review from aoustry and tbittar and removed request for tbittar August 10, 2026 10:05
Base automatically changed from api_thermal_heuristic to main August 11, 2026 12:08
return self.context.get_variable_structure(node.variable_id)

def lower_bound(self, node: LowerBoundNode) -> IndexingStructure:
return self.context.get_variable_structure(node.variable_id)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That's a bit a detail, but do you think the lower/upper bounds always have the same indexing structure as their related variable? I would say that this is not necessarily the case (ex: variable time-dep but bounds time-indep).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I agree, and more precisely, I think a lower/upper bound should always be "less granularly" indexed than the variable (the remark is valid for both time and scenario). Ie. the var can depend on time/scenario but not the bound. The reverse should not be possible (at least you think about a use case where we need that ?)

And if you agree on that, no checks are currently made within GemsPy, which may lead to some failures, see #267

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Then here, I think we should enrich the IdexingStructureProvider to also bear information about variable bounds indexing (clean way)

Otherwise simply using the variable indexing should be fine at the cost of possibly having some expressions with a constant timeseries (should work but not optimal)

@Juliette-Gerbaux Juliette-Gerbaux Aug 13, 2026

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.

I thought it was better to have the same indexing structure as the variable: since the heuristic can change the value of the bound and make it time-dependent, it seemed more intuitive for the heuristic to change only the value and not the indexing structure — but I'm open to changing that. The issue #267 would still be valid.

return self.context.get_variable_structure(node.variable_id)

def lower_bound(self, node: LowerBoundNode) -> IndexingStructure:
return self.context.get_variable_structure(node.variable_id)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I agree, and more precisely, I think a lower/upper bound should always be "less granularly" indexed than the variable (the remark is valid for both time and scenario). Ie. the var can depend on time/scenario but not the bound. The reverse should not be possible (at least you think about a use case where we need that ?)

And if you agree on that, no checks are currently made within GemsPy, which may lead to some failures, see #267

return self.context.get_variable_structure(node.variable_id)

def lower_bound(self, node: LowerBoundNode) -> IndexingStructure:
return self.context.get_variable_structure(node.variable_id)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Then here, I think we should enrich the IdexingStructureProvider to also bear information about variable bounds indexing (clean way)

Otherwise simply using the variable indexing should be fine at the cost of possibly having some expressions with a constant timeseries (should work but not optimal)

assert sum(_SPIL_FAST) == pytest.approx(255873)
assert total_output_sum(st, THERMAL_COMPONENTS, "non_prop_cost") == pytest.approx(
4640150
2273100

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do you get the same value in Antares legacy ?

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.

I had 2123660 in Antares legacy so I don't have the same value but this one is closer than the old one.

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.

3 participants