Add config obj replace schemas

[irm-codebase]

Compliments PR #704 by extending pydantic validation to other sections of model .yaml files

Summary of changes in this pull request

New schemas for the following:

• Model Math
• Dimension data (techs, nodes)
• Data Tables
• Model definition

This PR does not:

• Replace model_def_schema.yaml because it has parameter defaults. This needs to be removed in a subsequent PR.
• Integrate the use of the new schemas in type annotations. Mostly because the parameters / dims PR will affect this significantly.
• Replace preprocess/model_math.py with the new schema. Once again, the parmeters / dims PR should take care of this.

Reviewer checklist

• Test(s) added to cover contribution
• Documentation updated
• Changelog updated
• Coverage maintained or improved

[irm-codebase]

@brynpickering before continuing on this, we need to ensure we have a common vision on where / how stuff is checked, and the pros/cons of certain approaches. Particularly for places were our setup conflicts with pydantic's typing-based approach.

A key assumption, based on previous discussions, is that we are attempting to remove all schema .yaml files in favour of a pydantic approach. This would lead to a final CalliopeModel composed of several pydantic structures, each replacing one of our separate schemas (data_table_schema.yaml, math_schema.yaml...).

The final object would look like this, sort of:

class CalliopeModel(BaseModel):
    """Calliope model class."""

    model_config = {"title": "calliope model"}
    config: CalliopeConfig
    techs: dict[str, Tech]
    nodes: dict[str, Node]
    data_tables: dict[str, DataTable] | None = None
    parameters: Parameter | None = None  # this might go in Math later, here for now
    math: Math | None = None  # inserted by us using build.mode???
    overrides: None | dict[str, Self] = None

Open question 1: The role of pydantic in checking user model files

From a pure SW validation / data validation perspective: should we strive to catch invalid cases with pydantic as much as possible? To what extent should pydantic be used to ensure that the data going into later stages is 'good'? What is the effect of going either way?

Some examples of trivial checks with pydantic:

• Ensure all techs used in nodes.techs are defined in the default or in overrides.
• Run tests to ensure that all dimensions given in dims: sections (e.g., in techs, data_tables, etc) are present in a dimensions dictionary section or in overrides of said section.
• Ensuring that all parameter names used in techs or nodes sections are present in our definition or in user provided parameter definitions.
• Ensuring that parameters that go 'into' the optimisation have numeric values.
• etc

Open question 2: Where an when to introduce our basic parameter math and user extended math

At the moment a good chunk of our parameter defaults / checks is in model_def_schema.yaml, which we cram into model._data.attrs. pydantic might change this

• Should all these these defaults be in a pydantic model?
• How flexible should this be? Some values can be either parameters or variables depending on the mode (e.g, flow_cap).
• Hard-defining all these parameters within the pydantic model might prove to be too inflexible. Should we do so? An alternative would be just having our modes insert them into the math section of our configuration, and pydantic just checking that they are numeric (i.e., specific parameter names being in the pydantic model vs just in the dictionary that uses it).

Open question 3: How to support user-provided 'passthrough' settings without weakening validation

This stems from issue #709. Since pydantic is type based, our grouping of items in the dictionary matters quite a bit on how strong / weak our validation becomes. Our current flat structure has advantages when it comes to user simplicity, but at a possibly heavy cost of validation strength.

As an example, our current techs definition:

supply_gas:
    name: "Natural gas import"
    color: "#C98AAD"
    carrier_out: gas
    source_use_max: .inf
    flow_cap_max: 2000
    lifetime: 25
    cost_flow_cap:
      data: 1
      index: monetary
      dims: costs
    cost_flow_in:
      data: 0.025 # 2.5p/kWh gas price #ppt
      index: monetary
      dims: costs

This would require a Tech model structured like this. The big issue here is that you need to allow extras at the same level of our hard-defined attributes! I worry this will get messy fast. A less flat structure, which separates math parameters, code-bound values (e.g., carrier_out) and user 'extras' would be stronger, but less user friendly if done poorly.

class Tech(BaseModel):
    model_config = {
        "extra": "allow"
    }
    name: str
    color: str | None = None
    carrier_out: str
    flow_cap_max: PositiveReal  = float('inf')  # Assume we define PositiveReal some place else
    ...

[irm-codebase]

We can also keep all of the elements in CalliopeModel in separate places. It's only an illustration.

[brynpickering]

Thanks @irm-codebase - this is what I had in mind w.r.t. a move to pydantic! One thing I think it will offer us is the ability to remove AttrDict entirely 👀... We still need to be able to load our flavour of YAML, but that could be something we do with less maintenance overhead.

RE techs, we have a few weird parameters, but keep in mind that none of them are really "hard" requirements in terms of YAML definitions. You can define all that info in data tables and then have almost nothing defined in YAML. At that point pydantic's magic is useless to us anyway, hence why I wouldn't put too much focus on maximising the value of pydantic; we'll only ever apply it to a subset of the data when data tables are used. Data validation should wait a bit longer and be applied once YAML and data table info has been merged (i.e. on the xarray arrays). This means that no, parameter settings should not be in the pydantic model (as they shouldn't be in a YAML schema). Instead, they should be in a YAML definition (e.g. as given in #712) whose structure is described by a pydantic model.

Still, we have to define some things in pydantic under tech defs since they are weird parameters structures that we let through (namely, carrier_[in/out/export]) to make the user journey easier. Otherwise, we say that a tech setting/option/parameter can be any single value or an indexed parameter. These kinds of things can be part of the pydantic model and have some fancier cross-validation done if possible, at least in terms of the dimensions that have been defined?

RE the config options we include. We should consider how best to handle overrides and templates - should they be validated separately or should the pydantic model only be generated based on the resolved dict (all overrides and template data applied)?

[irm-codebase]

@brynpickering some replies (it's a bit difficult to follow which section you are referring to, but I'll try my best here).

RE techs, we have a few weird parameters, but keep in mind that none of them are really "hard" requirements in terms of YAML definitions. You can define all that info in data tables and then have almost nothing defined in YAML. At that point pydantic's magic is useless to us anyway, hence why I wouldn't put too much focus on maximising the value of pydantic; we'll only ever apply it to a subset of the data when data tables are used.

Understood. One important caveat of this is that we cannot easily offer some features that users might want (i.e., setting max/min ranges for a numeric parameter) without having to write this code ourselves. This is fine as long as it is understood.

If we ever wish to, we could re-arrange our parsing to happen after templates and data-tables are loaded to the dictionary (file -> our yaml loading -> complex pydantic -> backend). In this setup we would be able to rely on pydantic more.

For now, I will assume we'll go with the simplistic pydantic model.

Data validation should wait a bit longer and be applied once YAML and data table info has been merged (i.e. on the xarray arrays). This means that no, parameter settings should not be in the pydantic model (as they shouldn't be in a YAML schema). Instead, they should be in a YAML definition (e.g. as given in #712) whose structure is described by a pydantic model.

I think we pretty much agree on how this should be done, then.

Still, we have to define some things in pydantic under tech defs since they are weird parameters structures that we let through (namely, carrier_[in/out/export]) to make the user journey easier. Otherwise, we say that a tech setting/option/parameter can be any single value or an indexed parameter. These kinds of things can be part of the pydantic model and have some fancier cross-validation done if possible, at least in terms of the dimensions that have been defined?

This is the hard one. To answer the question(?) at the end: yes, but it depends. This comes back to my point on our current flat structure being troublesome. Checking certain things gets really hard when you have them all in a "salad" dictionary, even more so one can be extended by users.

As far as I can tell, this is the problem:

class CalliopeModel(BaseModel):
    """Calliope model class."""

    model_config = {"title": "calliope model"}
    config: CalliopeConfig
    techs: dict[str, Tech]
    ...

class TechFlat(BaseModel):
    model_config = {
        "extra": "allow"
    }
    # ANY other value will pass without checks! If we do not define it, we do not check it!
    # User 'extras' are a superset of what we could check, meaning that we cannot check non-specified items
    # We essentially do not check the **structure** of anything beyond hard-coded stuff (e.g., name, carrier_in)
    name: str
    carrier_in: str | None = None
    carrier_out: str | None = None
    carrier_export: str | None = None

class TechDivided(BaseModel):
    model_config = {
         "extra": "forbid"  
    }
    # May have anything as long as it's single values, to keep our IO simple
    extra: dict[str, float | str] | None = None
    # Structure is checked!
    params: dict[str, float]
    # Structure is checked!
    lookups: dict[str, str]

I am struggling a bit on how to properly setup a flat structure with passover values that does not end up feeling pointless due to the superset nature of user extras. Basically, any value can be there, even values that would not be used by some combination of our settings, with the extra ambiguity of not knowing if it is a user 'extra' is accidentally named as something important.

RE the config options we include. We should consider how best to handle overrides and templates - should they be validated separately or should the pydantic model only be generated based on the resolved dict (all overrides and template data applied)?

The answer depends on what we want to do.

• If we want to just validate the file, with no loss of information, then both overrides and tempaltes need to be accounted for in pydantic. In this approach, we never mess with the resulting dictionary before pydantic functions are done.
• If we want to validate the structure before it enters calliope, then we resolve these before running pydantic.

Both have pros / cons. I think the second is the easiest in our current setup.

I'd say second... solved by our yaml reading function?

[brynpickering]

Understood. One important caveat of this is that we cannot easily offer some features that users might want (i.e., setting max/min ranges for a numeric parameter) without having to write this code ourselves. This is fine as long as it is understood.

That would be dealt with by the parameter config being introduced in #712 . We can have min/max or a more general way to define validation rules that will act on the xarray dataset (using the format defined in https://github.com/calliope-project/calliope/blob/959fea7096ce47dfaf6886be1e1fa9445b92bc69/src/calliope/config/model_data_checks.yaml). This is what I mean about data validation that is beyond the scope of pydantic as it is no longer working on a dictionary of data, but arrays in an xarray dataset.

This comes back to my point on our current flat structure being troublesome.

I don't think it's the flat structure that causes the problem so much as allowing for user-defined keys. One option would be to consider dynamic fields. For instance, you can set type annotations for extra fields such that they have to at least follow a specific structure (as we define with our patternproperties currently). So the flat structure for a tech could be:

class TechFlat(BaseModel):
    model_config = {
        "extra": "allow"
    }
    __pydantic_extra__: dict[RegexStrWithExclusions, IndexedParam | bool | int | float | str] = Field(union_mode='left_to_right')
    base_tech: Literal[...]
    carrier_in: str | list[str] | None = None
    carrier_out: str | list[str] | None = None
    carrier_export: str | list[str] | None = None
    # + computed fields for to/from based on `base_tech`?

[brynpickering]

To update on the idea of using __pydantic_extra__, you could do this:

DATA_T = bool | int | float | str

class ConfigBaseModel(BaseModel):
     """A base class for creating pydantic models for Calliope configuration options."""
     _kwargs: dict = {}
     
     def update(self, update_dict: dict, deep: bool = False) -> Self:
         """Return a new iteration of the model with updated fields.
         Updates are validated and stored in the parent class in the `_kwargs` key.
         Args:
             update_dict (dict): Dictionary with which to update the base model.
             deep (bool, optional): Set to True to make a deep copy of the model. Defaults to False.
         Returns:
             BaseModel: New model instance.
         """
         new_dict: dict = {}
         # Iterate through dict to be updated and convert any sub-dicts into their respective pydantic model objects
         for key, val in update_dict.items():
             key_class = getattr(self, key)
             if isinstance(key_class, ConfigBaseModel):
                 new_dict[key] = key_class._update(val)
             else:
                 new_dict[key] = val
         updated = self.model_copy(update=new_dict, deep=deep)
         updated = self.model_validate(updated.model_dump())
         self._kwargs = update_dict
         return updated
     
     def _update(self, val):
         updated = self.update(val)
         self._kwargs = val
         return updated

class Param(ConfigBaseModel):
    """Uniform dictionairy for parameters."""
    data: DATA_T | list[bool | int | float | str]
    index: list[list[str]] = [[]]
    dims: list[str] = []

    def _update(self, val):
        if isinstance(val, dict):
            updated = self.update(val)
            self._kwargs = val
            return updated
        else:
            return val
        

class Tech(ConfigBaseModel):
    model_config = {
         "extra": "allow"
     }
    __pydantic_extra__: dict[str, Param | DATA_T] = Field(union_mode='left_to_right')
    base_tech: Literal["supply"]  # etc. 
    carrier_in: str | list[str] | None = None
    carrier_out: str | list[str] | None = None
    carrier_export: str | list[str] | None = None


class Techs(ConfigBaseModel):
    model_config = {
         "extra": "allow"
     }
    __pydantic_extra__: dict[str, Tech]


class Model(ConfigBaseModel):
    techs: Techs

Which would work to manage a dict like:

{"techs": {"foo": {"bar": {"data": 1}, "baz": "foobar", "base_tech": "supply", "carrier_in": ["elec", "heat"]}}}

And would give you appropriate dot notation afterwards (e.g. techs.foo.bar.carrier_in)

[brynpickering]

@irm-codebase I've edited my previous comment based on playing around with the update method to handle the fact that an update to the data might add/remove a Param class from a technology (I left in the _kwargs stuff but realise you probably have an updated approach to this).

[irm-codebase]

@brynpickering thanks for the update!
It is similar to the design I had in mind, but I was not aware of __pydantic_extra__...

Is forcing subdicts to update to their pydantic model necessary, though? I thought that was done automatically (or it seemed to be during my tests).

[brynpickering]

Is forcing subdicts to update to their pydantic model necessary, though? I thought that was done automatically (or it seemed to be during my tests).

I found that it just generated subdicts in my updated pydantic model, not nested pydantic models. If there's a way to do it without the extra lines I've added, then that's great!

[irm-codebase]

Is forcing subdicts to update to their pydantic model necessary, though? I thought that was done automatically (or it seemed to be during my tests).

I found that it just generated subdicts in my updated pydantic model, not nested pydantic models. If there's a way to do it without the extra lines I've added, then that's great!

@brynpickering seems like this can be achieved with revalidate_instances? See here

I'll see if I can get this to work on my updates.

[irm-codebase]

@brynpickering to keep things simple, I will change focus to merge this into main after PR #704 is complete.
This way we avoid more layered PRs

[jnnr]

Regarding the naming of link connections nodes:

• I have seen source, target sometimes.
• I like link_from, link_to.
• My overall favorite has not been mentioned: from_node, to_node.

[irm-codebase]

@jnnr I've gone with link_to / link_from for now, mostly because "link" is a term that is used quite often in our docs.

[irm-codebase]

@brynpickering I've ran into an 'edge case' triggered by mixing YAML and data table definitions.

Initially, I added some extra validation to the techs schema to detect incorrect technology definitions (i.e., not specifying base_tech, or not specifying carrier_in for a demand). However, these trigger false positives if you 'mix' loading some data from data tables and from YAML.

I'll unfortunately have to remove these tests (leaving the detection of this stuff to ModelDataFactory). This might be fine for now, but I think it indicates that we are not being 'concise' enough on how we allow users to define stuff.

Shouldn't we differentiate between 'structural' model definition (i.e., base_tech, carrier_in) and data definition (cost_flow_cap, flow_in_eff), disallowing to load the first from data tables?

[irm-codebase]

@brynpickering ready for review!

Although this is a lengthy PR, the line difference number that github is giving is kind of deceptive. Most of it was due to a .csv file change.

I've updated the first comment of the PR to reflect the changes and omissions of this PR. In general:

• model_def_schema.yaml remains because it contains parameter defaults.
• preprocess/model_math.py likely needs to be merged into the math schema. For now, I kept them separate because the params / dims update will impact this and this PR is long enough already.

I've been as thorough as I can, but this is still a rather critical change. We need to be careful, so any comments / concerns are welcome.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.40%. Comparing base (5a2b28a) to head (11dd2f8).
Report is 11 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #717      +/-   ##
==========================================
+ Coverage   96.22%   96.40%   +0.18%     
==========================================
  Files          30       35       +5     
  Lines        4181     4397     +216     
  Branches      591      599       +8     
==========================================
+ Hits         4023     4239     +216     
  Misses         68       68              
  Partials       90       90              

Files with missing lines	Coverage Δ
src/calliope/backend/__init__.py	100.00% <ø> (ø)
src/calliope/backend/backend_model.py	97.99% <100.00%> (+<0.01%)	⬆️
src/calliope/backend/gurobi_backend_model.py	95.29% <100.00%> (ø)
src/calliope/backend/latex_backend_model.py	96.89% <100.00%> (ø)
src/calliope/backend/pyomo_backend_model.py	98.11% <100.00%> (ø)
src/calliope/backend/where_parser.py	98.20% <100.00%> (ø)
src/calliope/cli.py	81.95% <100.00%> (+0.28%)	⬆️
src/calliope/model.py	97.52% <100.00%> (+0.01%)	⬆️
src/calliope/postprocess/postprocess.py	98.18% <100.00%> (ø)
src/calliope/preprocess/data_tables.py	100.00% <100.00%> (ø)
... and 11 more

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[brynpickering]

Pydanic models look good! Our test suite really does not cleaning. Need to work out a better way to have minimal models to load and check. This is a task for #240 though.

[sjpfenninger]

Scanned this and it looks good to me, nothing to add on top of what Bryn noted!

[irm-codebase]

I've added most of your suggestions / fixes @brynpickering.
I left a comment for those that I did not implement.

[brynpickering]

One minor comment on the snakecase regex, otherwise good to go from my perspective!