Skip to content

Feature/robust checks#229

Merged
aoustry merged 39 commits into
developfrom
feature/robust-checks
Jun 11, 2026
Merged

Feature/robust checks#229
aoustry merged 39 commits into
developfrom
feature/robust-checks

Conversation

@tbittar

@tbittar tbittar commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR strengthens correctness guarantees of the library-resolution and simulation pipeline in three areas: the linearity check in constraints and objectives is generalized from dual/reduced_cost detection to a proper non-linearity test; two new semantic rules on port-field expressions are enforced at library-resolve time; and the system-level `sum_connections` robustness check is made more general.


Behavior changes

1. Constraint / objective linearity check — `_forbid_dual_or_rc` → `_forbid_nonlinear`

The previous guard (`contains_dual_or_reduced_cost`) only caught `dual()` and `reduced_cost()`. It is replaced by `is_linear()`, which correctly rejects all non-linear expressions.

Expression in a binding constraint or objective Before After
`dual(c)` `ValueError` (detected as dual/rc) `ValueError` (non-linear)
`reduced_cost(x)` `ValueError` (detected as dual/rc) `ValueError` (non-linear)
`abs(x)` with `x` a variable Accepted (not dual/rc) `ValueError` (non-linear)
`max(x, y)` with variables Accepted (not dual/rc) `ValueError` (non-linear)
`abs(a)` / `max(a, b)` with parameters only Accepted Accepted (constant, degree 0)

The same generalisation is applied in `optimization.py`: port-field definitions that are non-linear (not just dual/rc) are now correctly skipped during the LP build phase and deferred to the extra-output post-solve evaluator.

2. New rule: bare `port.field` is not allowed outside `sum_connections`

A bare `PortFieldNode` (e.g. `balance_port.flow`) in any binding constraint, plain constraint, extra-output, or objective contribution now raises a `ValueError` at `resolve_library()` time. `sum_connections(...)` is not recursed into, so the `PortFieldNode` inside remains valid.

Expression Context Before After
`balance_port.flow >= 0` binding constraint Accepted (degree 1, linear) `ValueError`
`balance_port.price >= 0` binding constraint Accepted `ValueError`
`balance_port.flow = withdrawal` plain constraint Accepted `ValueError`
`sum_connections(balance_port.flow) >= 0` any Accepted Accepted (no change)

3. New rule: `sum_connections` cannot refer to a port field defined in the current model

If a model's `port-field-definitions` already provides a definition for `port.field`, any expression that contains `sum_connections(port.field)` — in a binding constraint, plain constraint, extra-output, or objective — now raises a `ValueError` at `resolve_library()` time.

Expression Condition Before After
`sum_connections(p.f)` in a binding constraint `p.f` defined in this model Silently accepted `ValueError`
`sum_connections(p.f)` in an extra-output `p.f` defined in this model Silently accepted `ValueError`
`sum_connections(p.f)` in a constraint `p.f` defined in this model Silently accepted `ValueError`
`sum_connections(p.f)` anywhere `p.f` not defined in this model Accepted Accepted (no change)

4. System-level `sum_connections` robustness — `_check_no_dual_rc_sum_connections` → `_check_linear_sum_connections`

The system-level guard that prevents aggregating a non-linear port-field definition via `sum_connections` previously only detected `dual()`/`reduced_cost()`. It now uses `is_linear()`, catching all non-linear definitions.

Master port-field definition Before After
Contains `dual()` or `reduced_cost()` `ValueError` at connection time `ValueError` at connection time
Contains `abs(x)`, `max(x,y)`, etc. on variables Silently accepted (wrong) `ValueError` at connection time
Linear Accepted Accepted (no change)

Other changes

  • `predicates.py` removed; `uses_sum_connections_on.py` (renamed from `predicates.py`) is now the single-purpose module for that visitor.
  • `port_resolver.py` removed (superseded).
  • `model_port_definition_ok.yml` and `model_port_definition_ko.yml` fixtures updated: bare port field reference replaced with `sum_connections(...)`, consistent with Rule 2.
  • New tests in `test_lib_parsing.py`: acceptance/rejection matrix for all expression contexts (variable bounds, objectives, constraints, extra-outputs), plus 6 dedicated tests for the two new port-field rules.
  • New tests in `test_components_parsing.py`: `sum_connections` linearity checks at connection time.

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

@tbittar tbittar force-pushed the feature/robust-checks branch from 007e76d to f365c07 Compare June 10, 2026 15:53
@tbittar tbittar force-pushed the feature/dual-reduced-cost-clean branch from 91d6ad6 to 85626cd Compare June 10, 2026 15:55
@tbittar tbittar force-pushed the feature/robust-checks branch from f365c07 to 56488ee Compare June 10, 2026 15:55
@tbittar tbittar mentioned this pull request Jun 11, 2026
3 tasks
@tbittar tbittar linked an issue Jun 11, 2026 that may be closed by this pull request
20 tasks
Base automatically changed from feature/dual-reduced-cost-clean to develop June 11, 2026 09:40
Comment thread tests/unittests/system_parsing/test_components_parsing.py
@tbittar tbittar force-pushed the feature/robust-checks branch from adaebe2 to 05d6819 Compare June 11, 2026 10:56
@aoustry aoustry merged commit f5cc138 into develop Jun 11, 2026
2 checks passed
@aoustry aoustry deleted the feature/robust-checks branch June 11, 2026 11:48
tbittar added a commit that referenced this pull request Jun 11, 2026
* feat/ add operators abs & round (#217)

* Add abs and round unary operators (#216)

Two new unary operators that mirror the floor/ceil pattern:
- abs(x): absolute value
- round(x): banker's rounding (round-half-to-even), matching np.round
  and Python 3's built-in round.

Like floor/ceil, both operators have degree 0 when their argument has
degree 0, so they are usable inside constraints, binding-constraints,
objective contributions, and variable lower/upper bounds whenever the
argument is constant (parameters and literals). Inside extra-outputs
they may wrap any expression — including ones depending on decision
variables — since extra-outputs are evaluated as numeric xr.DataArrays
post-solve.

* feat(readme): modernize the design of the readme file (#223)

* Restyle README with modern layout

* Add GEMS favicon next to 'The GEMS framework' heading

* Remove top logo image from README header

* Replace 'no-code' with 'low-code' in README

* Use GEMS favicon in quick-link nav

* Vendor GEMS favicon under docs/images and reference it locally

* Add uv install instructions to README

* Prepare develop/ for release v0.1.2 (#225)

* Prepare release v0.1.2

Bump version from 0.1.1 to 0.1.2 in pyproject.toml and uv.lock, and
finalize the CHANGELOG with the abs/round operators and the README
modernization that landed since 0.1.1.

https://claude.ai/code/session_01AfUVdznMY9SayUVt5f3K4T

* Update v0.1.2 release date to 2026-06-11

https://claude.ai/code/session_01AfUVdznMY9SayUVt5f3K4T

---------

Co-authored-by: Claude <[email protected]>

* Fix imports in doc (#227)

* Update agents.md (#228)

* Handle dual and reduced cost (#221)

* Initial commit

* delete unnecessary issue templae

* exclude compatibility file

* remove compatibility file from issue templates

* delete changelog file

* fix step 9 in issue templates

* add notify workflow

* add token for antares legacy converter

* add issue creation for GemsViewsBuilder repo

* fix/Update links to GEMS repo (#219)

* update links to GEMS repo

* transparent pictures

* fix png image link

* scheme -> schema

* remove duplicate ci (#222)

* Add tests

* WIP

* Use visitor pattern

* Use visitor pattern

* Support xpress, gurobi

* Fix solver specific test and mypy

* Formatting

* Pre-commit consistency

* Reorganize tests

* Update docs

* Update xhangelog

* Formatting

* Complete new visitor implementation following rebase

* Fix usage in ports

* Update tests

* Apply suggestion from @aoustry

* update docstring

---------

Co-authored-by: nikolaredstork <[email protected]>
Co-authored-by: Guillaume_RTEi <[email protected]>
Co-authored-by: Antoine Oustry, PhD <[email protected]>

* feat(library <-> taxonomy check) (#214)

* feat(properties):  add properties field to support system parsing

* fix(tests): remove unnecessary blank line in test_systemschema_from_file.py

* refactor: remove unused import PortsConnection from resolve_components.py

* feat(schema): add optional taxonomy_category field to ModelSchema and implement corresponding unit test for YAML parsing

* Apply suggestion from @dusanparipovic

* Update gemspy version to 0.1.0 and clean up whitespace in ModelSchema taxonomy_category field

* feat(changelog): add new features for component properties and taxonomy category

- Introduced optional `properties` for components in `system.yml`, allowing key/value pairs that are normalized into a dictionary.
- Added optional `taxonomy-category` field for models in library YAML files, accessible via `ModelSchema.taxonomy_category`.
- Updated documentation to reflect these changes and provided examples in the user guide.

* fix(docs): clarify properties key naming in documentation and tests

- Updated documentation to replace "key" with "id" in the context of component properties in `system.yml`.
- Adjusted test assertions to reflect the change from "key" to "id" for consistency with the updated documentation.
- Ensured that error messages for duplicate properties also refer to "id" instead of "key".

* feat(study): introduce Study class to encapsulate System and DataBase

- Added a new `Study` dataclass that combines `System` and `DataBase`, centralizing consistency checks.
- Updated `build_problem()` and `build_decomposed_problems()` to accept `Study` directly.
- Refactored related functions and tests to utilize the new `Study` structure, ensuring seamless integration.
- Removed redundant parameters and streamlined the API for better clarity and usability.

* feat(changelog): add changelog

* fix(ruff): fix ruff format on new files

* Apply suggestion from @tbittar

Co-authored-by: tbittar <[email protected]>

* Apply suggestion from @tbittar

Co-authored-by: tbittar <[email protected]>

* Apply suggestion from @tbittar

Co-authored-by: tbittar <[email protected]>

* Apply suggestion from @aoustry

* Apply suggestion from @aoustry

* Update CHANGELOG.md

* Update input documentation on duplicate id handling

Clarified that duplicate ids for properties are rejected.

* fix(taxonomy): import ConstraintSchema and PortFieldDefinitionSchema (#230)

Fixes mypy [name-defined] errors on TaxonomyCategory fields.

Co-authored-by: Claude <[email protected]>

* Address pending review comments: taxonomy field-group validation + model-declared properties (#231)

* feat(taxonomy/models): validate all taxonomy field groups and model-declared properties

Address pending review comments on PR #214:

- check_library_against_taxonomy now validates every field group declared
  in a taxonomy category (variables, parameters, ports, port-field-definitions,
  constraints, binding-constraints, extra-outputs, properties) instead of ports only.
- Add an optional `properties` list to ModelSchema and Model (declared keys).
  When resolving a component, every property key declared by its model must be
  present in the component's properties; extra undeclared properties are allowed.
- Add per-field-group taxonomy tests and component model-property tests.
- Remove redundant tests in test_systemschema_from_file.py (covered by
  test_components_parsing.py).


* docs(changelog): fold property/taxonomy notes into 0.1.2

Integrate the model-properties and taxonomy field-group changes into the
upcoming 0.1.2 section instead of a separate Unreleased block, merging with
the existing close entries to avoid redundancy.

* test: move model-property library tests to lib_parsing (#232)

The tests asserting that a library exposes model-declared properties
(parse + resolve) only exercise library parsing/resolution and do not
involve system/component resolution, so they belong in the lib_parsing
test file rather than test_components_parsing.py.

---------

Co-authored-by: Antoine Oustry, PhD <[email protected]>
Co-authored-by: tbittar <[email protected]>

* Feature/robust checks (#229)

---------

Co-authored-by: Claude <[email protected]>
Co-authored-by: Juliette-Gerbaux <[email protected]>
Co-authored-by: tbittar <[email protected]>
Co-authored-by: nikolaredstork <[email protected]>
Co-authored-by: Guillaume_RTEi <[email protected]>
Co-authored-by: Dušan <[email protected]>
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.

[GP-02] Unauthorized port use in expressions

5 participants