Refactor/yaml domain components #638
Conversation
jsolaas
force-pushed
the
refactor/yaml-domain-components
branch
from
5ac64ad to
561a0f5
Compare
| class Component(abc.ABC): | ||
| """ | ||
| Common info for a component in the model | ||
| """ | ||
|
|
||
| @property | ||
| @abc.abstractmethod | ||
| def id(self) -> ComponentID: ... | ||
|
|
||
| @property | ||
| @abc.abstractmethod | ||
| def name(self) -> str: ... | ||
|
|
||
| @abc.abstractmethod | ||
| def get_node_info(self) -> NodeInfo: ... | ||
|
|
||
| @abc.abstractmethod | ||
| def get_ecalc_model_result(self) -> Optional[EcalcModelResult]: | ||
| """ | ||
| TODO: We should look into removing this generic result, as it does not makes sense to provide the same result for all types of components. | ||
| """ | ||
| ... | ||
|
|
||
|
|
||
| class PowerProvider(Component, abc.ABC): | ||
| """ | ||
| Provider that can deliver a power requirement | ||
| """ | ||
|
|
||
| @abc.abstractmethod | ||
| def provide_power(self, power_requirement: List[TimeSeriesStreamDayRate]): ... | ||
|
|
||
|
|
||
| class Emitter(Component, abc.ABC): | ||
| """ | ||
| Something that causes emissions | ||
| """ | ||
|
|
||
| @abc.abstractmethod | ||
| def get_emissions(self, period: Period = None) -> Dict[str, EmissionResult]: ... | ||
|
|
||
|
|
||
| class PowerConsumer(Component, abc.ABC): | ||
| """ | ||
| A consumer with a power requirement | ||
| """ | ||
|
|
||
| @abc.abstractmethod | ||
| def get_power_requirement(self, period: Period = None) -> TimeSeriesStreamDayRate: ... | ||
|
|
||
|
|
||
| class FuelConsumer(Emitter, abc.ABC): | ||
| """ | ||
| An abstraction for provider and consumer that isn't able to give power requirement | ||
| """ | ||
|
|
||
| @abc.abstractmethod | ||
| def get_fuel_usage(self, period: Period = None) -> TimeSeriesStreamDayRate: ... | ||
|
|
||
|
|
||
| class ProcessGraph: | ||
| """ | ||
| A set of components that should be evaluated together, but reported as separate components. | ||
| """ | ||
|
|
||
| @abc.abstractmethod | ||
| def get_components(self) -> List[Component]: ... | ||
|
|
||
| @abc.abstractmethod | ||
| def evaluate(self) -> None: ... | ||
|
|
||
|
|
||
| # TODO: Refactor ComponentGraph to EnergyGraph, protocol and composition instead of inherit from graph. | ||
| # EnergyGraph should know about ProcessGraph. | ||
|
|
There was a problem hiding this comment.
This should be discussed. This is the interfaces we implement for different components. Currently asset and installation don't implement an interface, so the ComponentGraph is a bit inconsistent. We should figure out which types of components we have, or implement an alternative interface.
There was a problem hiding this comment.
good point. At the same time asset and installation is currently only "containers", and not a part of a network per se. However wrt. power networks and inter-networks they should play a role. I think it makes sense to focus on what is "below installation" here, as the installation and asset currently just "aggregates" the data ...
| ) | ||
|
|
||
| def get_power_requirement(self, period: Period = None) -> TimeSeriesStreamDayRate: | ||
| consumer_result = self._core_consumer.evaluate(self._expression_evaluator) |
There was a problem hiding this comment.
I think we should aim to remove both dto and core consumer here, and rather treat the models as core.
| self._ecalc_model_result = consumer_result | ||
| return consumer_result.component_result.power | ||
|
|
||
| def get_ecalc_model_result(self) -> EcalcModelResult: |
There was a problem hiding this comment.
I also think we should aim to remove EcalcModelResult, instead provide specific results such as Streams, fuel usage, power usage, emissions. Per equipment results should also be considered. Instead of having EcalcModelResult for FuelConsumer that contains both fuel and power usage, we could have two sets of results, one that represents the turbine, and another that represents the compressor. Turbine naturally has fuel usage, compressor has power usage. If the fuel consumer doesn't provide power usage, there's only a turbine result.
We should think about this when defining the different types of components we have. each component might contain several components in the result context, but not in the evaluation context (Similar to ProcessGraph/ConsumerSystem).
| def get_node_info(self) -> NodeInfo: | ||
| return NodeInfo( |
There was a problem hiding this comment.
A better name? Could also include category? What else?
| class ReferenceService(Protocol): | ||
| def get_fuel_reference(self, reference: str) -> FuelType: ... | ||
|
|
||
| def get_generator_set_model(self, reference: str) -> GeneratorSetSampled: ... | ||
|
|
||
| def get_compressor_model(self, reference: str) -> CompressorModel: ... | ||
|
|
||
| def get_pump_model(self, reference: str) -> PumpModel: ... | ||
|
|
||
| def get_tabulated_model(self, reference: str) -> TabulatedData: ... |
There was a problem hiding this comment.
Should look into creating abstractions here, abc or protocol. Maybe split CompressorModel, as I think it is difficult to define a common interface for all the different models.
Creating classes wrapping dto and core, similar to this PR. I think...
|
|
||
| def get_compressor_model(self, reference: str) -> CompressorModel: | ||
| model = self._get_model_reference(reference, "compressor model") | ||
| if not isinstance(model, get_args(CompressorModel)): |
There was a problem hiding this comment.
get_args since it's a union..
| @@ -39,7 +39,7 @@ class YamlInstallation(YamlBase): | |||
| ) | |||
| category: InstallationUserDefinedCategoryType = CategoryField(None) | |||
| hydrocarbon_export: YamlTemporalModel[YamlExpressionType] = Field( | |||
| None, | |||
| 0, | |||
There was a problem hiding this comment.
Setting defaults in yaml makes it more clear and easier to use the property.
Init work to remove dtos by wrapping dto and mapping in (yaml) domain classes.
jsolaas
force-pushed
the
refactor/yaml-domain-components
branch
from
561a0f5 to
1b64069
Compare
Have you remembered and considered?
docs/docs/changelog/next.md)docs/docs/migration_guides/)BREAKING:in footer or!in header, if breakingECALC-XXXX)Why is this pull request needed?
This pull request is needed to be able to remove dtos. Dtos should be removed to improve flexibility by defining interfaces (abc/protocols) and depend on those interfaces instead of specific classes.
The responsibilities of different classes should also become more clear. I.e. Yaml-classes should make sure the yaml is valid, these classes should be able to depend on valid yaml classes, and further validate the combination of yaml and resources.
ReferenceService should also help in defining the different references we can have. Interfaces should be implemented here also, and classes implementing them, instead of pydantic.
Issues related to this change: