Skip to content

feat(library <-> taxonomy check)#214

Merged
aoustry merged 23 commits into
developfrom
models-taxonomy-check
Jun 11, 2026
Merged

feat(library <-> taxonomy check)#214
aoustry merged 23 commits into
developfrom
models-taxonomy-check

Conversation

@dusanparipovic

Copy link
Copy Markdown
Contributor

No description provided.

… implement corresponding unit test for YAML parsing
…my 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.
- 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".
- 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.
@dusanparipovic

Copy link
Copy Markdown
Contributor Author

Open questions:

  • This PR opens again question about shared library which will be used across our gems eco system.
  • In fact we have duplicate of class for Taxonomy inside GVB and Gemspy.
  • Also this PR opens question: do we want to be able to have multiple taxonomies and do we want to reference it inside library file.
  • For more details see issue inside GVB

@dusanparipovic

Copy link
Copy Markdown
Contributor Author

Open questions:

  • This PR opens again question about shared library which will be used across our gems eco system.
  • In fact we have duplicate of class for Taxonomy inside GVB and Gemspy.
  • Also this PR opens question: do we want to be able to have multiple taxonomies and do we want to reference it inside library file.
  • For more details see issue inside GVB

@aoustry @tbittar

@tbittar tbittar left a comment

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.

  • You need to do more complete checks between the taxonomy and models (variables, parameters, constraints, properties, extra-outputs)
  • The models should declare a list of properties such as :
model: thermal
   properties:
        - id: carrier
        - id: technology

We need to check that each model declares the required properties from the taxonomy (but it could declare additional ones)

  • If a model defines a property, it should be defined for each component of this model within the system file. We need to add a check for this. A component in the system may define additional properties that are not required by the model

I think we should keep this PR on the checks model <-> taxonomy for everything but the properties and handle the work on the properties (which may modify the models, systems and tests) in another PR (check taxonomy <-> library and library <-> system)

Comment thread docs/user-guide/inputs.md Outdated
Comment thread docs/user-guide/inputs.md Outdated
Comment on lines +61 to +62
At resolution time (`resolve_system` / `load_study`), this list is normalized into
a `dict[str, str]` stored on the resolved `Component`. Duplicate ids are rejected.

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 we need these technical details here ? Only mentioning that duplicated ids are forbidden is sufficient ?

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.

Indeed we can remove those two line

Comment thread docs/user-guide/inputs.md Outdated
Comment thread docs/CHANGELOG.md Outdated
Comment thread docs/CHANGELOG.md Outdated
Comment thread src/gems/model/taxonomy.py Outdated
Comment thread src/gems/model/taxonomy.py Outdated
)

category = categories[cat_id]
model_port_ids = {p.id for p in model_schema.ports}

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.

You need to check more than just the ports, see above comment

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.

Indeed, you need to check all the fields declared in the taxonomy-category:
variables
parameters
ports
port_field_definitions:
constraints
binding_constraints
extra_outputs
properties

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.

These tests seem redundant (and less complete) than the one in test_components_parsing.py. Should we remove them ?

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.

Ok!

Comment thread src/gems/model/taxonomy.py
- id: balance_port
type: flow
""")
check_library_against_taxonomy(lib, taxonomy) # must not raise

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.

Add more tests to check the presence of more types of fields (variables, parameters, constraints, port-field-definition, etc...)

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.

(see open comment on taxonomy.py)

aoustry and others added 4 commits June 10, 2026 17:53
@aoustry

aoustry commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator
  • You need to do more complete checks between the taxonomy and models (variables, parameters, constraints, properties, extra-outputs)
  • The models should declare a list of properties such as :
model: thermal
   properties:
        - id: carrier
        - id: technology

We need to check that each model declares the required properties from the taxonomy (but it could declare additional ones)

  • If a model defines a property, it should be defined for each component of this model within the system file. We need to add a check for this. A component in the system may define additional properties that are not required by the model

I think we should keep this PR on the checks model <-> taxonomy for everything but the properties and handle the work on the properties (which may modify the models, systems and tests) in another PR (check taxonomy <-> library and library <-> system)

@tbittar I think that model <-> taxonomy and library <-> taxonomy in fact refer to the same checks, no ?

@tbittar

tbittar commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator
  • You need to do more complete checks between the taxonomy and models (variables, parameters, constraints, properties, extra-outputs)
  • The models should declare a list of properties such as :
model: thermal
   properties:
        - id: carrier
        - id: technology

We need to check that each model declares the required properties from the taxonomy (but it could declare additional ones)

  • If a model defines a property, it should be defined for each component of this model within the system file. We need to add a check for this. A component in the system may define additional properties that are not required by the model

I think we should keep this PR on the checks model <-> taxonomy for everything but the properties and handle the work on the properties (which may modify the models, systems and tests) in another PR (check taxonomy <-> library and library <-> system)

@tbittar I think that model <-> taxonomy and library <-> taxonomy in fact refer to the same checks, no ?

Yes I was trying to say that we could separate in 2 PRs :

  • Checks library / taxonomy for everything except properties
  • Implement properties and add checks for library / taxonomy on the properties and on library / system for required properties
    I have not followed the last evolution of these PRs so my comment may not be relevant anymore

…del-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).

https://claude.ai/code/session_014gf1DB7zMKwddW2K6HkU8a

* 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.
@aoustry

aoustry commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator
  • You need to do more complete checks between the taxonomy and models (variables, parameters, constraints, properties, extra-outputs)
  • The models should declare a list of properties such as :
model: thermal
   properties:
        - id: carrier
        - id: technology

We need to check that each model declares the required properties from the taxonomy (but it could declare additional ones)

  • If a model defines a property, it should be defined for each component of this model within the system file. We need to add a check for this. A component in the system may define additional properties that are not required by the model

I think we should keep this PR on the checks model <-> taxonomy for everything but the properties and handle the work on the properties (which may modify the models, systems and tests) in another PR (check taxonomy <-> library and library <-> system)

@tbittar I think that model <-> taxonomy and library <-> taxonomy in fact refer to the same checks, no ?

Yes I was trying to say that we could separate in 2 PRs :

  • Checks library / taxonomy for everything except properties
  • Implement properties and add checks for library / taxonomy on the properties and on library / system for required properties
    I have not followed the last evolution of these PRs so my comment may not be relevant anymore

in the end, I did all of this in this PR since this was quite limited...

@aoustry aoustry requested a review from tbittar June 11, 2026 07:59
Comment thread docs/CHANGELOG.md Outdated
Comment thread tests/unittests/system_parsing/test_components_parsing.py Outdated
aoustry added 2 commits June 11, 2026 12:13
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.
@aoustry aoustry merged commit c2d0412 into develop Jun 11, 2026
2 checks passed
@aoustry aoustry deleted the models-taxonomy-check branch June 11, 2026 10:29
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.

3 participants