-
Notifications
You must be signed in to change notification settings - Fork 94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add config object to keep config in sync at all times #704
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,17 +26,13 @@ | |
import numpy as np | ||
import xarray as xr | ||
|
||
from calliope import exceptions | ||
from calliope import config, exceptions | ||
from calliope.attrdict import AttrDict | ||
from calliope.backend import helper_functions, parsing | ||
from calliope.exceptions import warn as model_warn | ||
from calliope.io import load_config | ||
from calliope.preprocess.model_math import ORDERED_COMPONENTS_T, CalliopeMath | ||
from calliope.util.schema import ( | ||
MODEL_SCHEMA, | ||
extract_from_schema, | ||
update_then_validate_config, | ||
) | ||
from calliope.util.schema import MODEL_SCHEMA, extract_from_schema | ||
Comment on lines
+29
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An issue here is that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think yes. The switch to parameters in math would drastically reduce the need for a massive model definition schema. Then the rest might be best blended with our TypedDicts that we use (which pydantic can translate directly to a model to validate against, reducing duplication of schema/type definitions in our code!) |
||
|
||
if TYPE_CHECKING: | ||
from calliope.backend.parsing import T as Tp | ||
|
@@ -69,20 +65,20 @@ class BackendModelGenerator(ABC): | |
_PARAM_UNITS = extract_from_schema(MODEL_SCHEMA, "x-unit") | ||
_PARAM_TYPE = extract_from_schema(MODEL_SCHEMA, "x-type") | ||
|
||
def __init__(self, inputs: xr.Dataset, math: CalliopeMath, **kwargs): | ||
def __init__( | ||
self, inputs: xr.Dataset, math: CalliopeMath, build_config: config.Build | ||
): | ||
"""Abstract base class to build a representation of the optimisation problem. | ||
|
||
Args: | ||
inputs (xr.Dataset): Calliope model data. | ||
math (CalliopeMath): Calliope math. | ||
**kwargs (Any): build configuration overrides. | ||
build_config: Build configuration options. | ||
""" | ||
self._dataset = xr.Dataset() | ||
self.inputs = inputs.copy() | ||
self.inputs.attrs = deepcopy(inputs.attrs) | ||
self.inputs.attrs["config"]["build"] = update_then_validate_config( | ||
"build", self.inputs.attrs["config"], **kwargs | ||
) | ||
self.config = build_config | ||
self.math: CalliopeMath = deepcopy(math) | ||
self._solve_logger = logging.getLogger(__name__ + ".<solve>") | ||
|
||
|
@@ -200,6 +196,7 @@ def _check_inputs(self): | |
"equation_name": "", | ||
"backend_interface": self, | ||
"input_data": self.inputs, | ||
"build_config": self.config, | ||
"helper_functions": helper_functions._registry["where"], | ||
"apply_where": True, | ||
"references": set(), | ||
|
@@ -246,7 +243,7 @@ def add_optimisation_components(self) -> None: | |
# The order of adding components matters! | ||
# 1. Variables, 2. Global Expressions, 3. Constraints, 4. Objectives | ||
self._add_all_inputs_as_parameters() | ||
if self.inputs.attrs["config"]["build"]["pre_validate_math_strings"]: | ||
if self.config.pre_validate_math_strings: | ||
self._validate_math_string_parsing() | ||
for components in typing.get_args(ORDERED_COMPONENTS_T): | ||
component = components.removesuffix("s") | ||
|
@@ -399,7 +396,7 @@ def _add_all_inputs_as_parameters(self) -> None: | |
if param_name in self.parameters.keys(): | ||
continue | ||
elif ( | ||
self.inputs.attrs["config"]["build"]["mode"] != "operate" | ||
self.config.mode != "operate" | ||
and param_name | ||
in extract_from_schema(MODEL_SCHEMA, "x-operate-param").keys() | ||
): | ||
|
@@ -606,17 +603,21 @@ class BackendModel(BackendModelGenerator, Generic[T]): | |
"""Calliope's backend model functionality.""" | ||
|
||
def __init__( | ||
self, inputs: xr.Dataset, math: CalliopeMath, instance: T, **kwargs | ||
self, | ||
inputs: xr.Dataset, | ||
math: CalliopeMath, | ||
instance: T, | ||
build_config: config.Build, | ||
) -> None: | ||
"""Abstract base class to build backend models that interface with solvers. | ||
|
||
Args: | ||
inputs (xr.Dataset): Calliope model data. | ||
math (CalliopeMath): Calliope math. | ||
instance (T): Interface model instance. | ||
**kwargs: build configuration overrides. | ||
build_config: Build configuration options. | ||
""" | ||
super().__init__(inputs, math, **kwargs) | ||
super().__init__(inputs, math, build_config) | ||
self._instance = instance | ||
self.shadow_prices: ShadowPrices | ||
self._has_verbose_strings: bool = False | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ | |
from calliope.exceptions import BackendError | ||
|
||
if TYPE_CHECKING: | ||
from calliope import config | ||
from calliope.backend.backend_model import BackendModel | ||
Comment on lines
19
to
21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The need for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I only put it in there because its use in the parser is purely as a type hint. It's a way to show that it has no other purpose as well as to avoid conflicts (in this case, it doesn't create a conflict) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer if it was moved to a regular import. A conflict is a code smell that indicates that the software could be arranged better, and this may hide those cases. |
||
|
||
|
||
|
@@ -34,6 +35,7 @@ class EvalAttrs(TypedDict): | |
helper_functions: dict[str, Callable] | ||
apply_where: NotRequired[bool] | ||
references: NotRequired[set] | ||
build_config: config.Build | ||
|
||
|
||
class EvalWhere(expression_parser.EvalToArrayStr): | ||
|
@@ -118,9 +120,7 @@ def as_math_string(self) -> str: # noqa: D102, override | |
return rf"\text{{config.{self.config_option}}}" | ||
|
||
def as_array(self) -> xr.DataArray: # noqa: D102, override | ||
config_val = ( | ||
self.eval_attrs["input_data"].attrs["config"].build[self.config_option] | ||
) | ||
config_val = getattr(self.eval_attrs["build_config"], self.config_option) | ||
|
||
if not isinstance(config_val, int | float | str | bool | np.bool_): | ||
raise BackendError( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an area where the old approach and the new
pydantic
may be at odds.Case
_
is spurious, since pydantic should catch wrong settings beforehand, no?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, you're right! Actually it was always kinda spurious as we always call this with info from the config and the config was being validated against the schema which only had two backend options.