418 asset max size and esdl attribute used#419
Conversation
src/mesido/esdl/esdl_heat_model.py
Outdated
| if len(asset.attributes["constraint"]) > 1: | ||
| logger.warning( | ||
| f"More than 1 range constraint has been specified to " | ||
| f"asset named {asset.name}, currenlty only the 1st constraint is being used" | ||
| ) |
There was a problem hiding this comment.
What happens with assets that both have a profile constraint and a max power constraint. This would for instance occur on how aquathermal assets have to be implemented.
There was a problem hiding this comment.
ahhhh very nice catch. Thanks. Code has been updated to check type of constraints now
There was a problem hiding this comment.
Several changes made to cater for this. Now checking if a constraint type exists etc
src/mesido/esdl/esdl_heat_model.py
Outdated
| logger.exit( # still to decide error vs warning | ||
| "Expected a range contraint (upper size limit) for asset named " | ||
| f"{asset.name}, but none has been specified." | ||
| ) |
There was a problem hiding this comment.
I would prefer a warning, as it might otherwise also break how other assets are being used, or in other workflows.
src/mesido/esdl/esdl_heat_model.py
Outdated
| if not max_value_range > 0.0: | ||
| get_potential_errors().add_potential_issue( | ||
| MesidoAssetIssueType.ASSET_UPPER_LIMIT, | ||
| asset.id, | ||
| f"Asset named {asset.name}: The maximum value in the range " | ||
| f"constraint for attribute {max_size_attribute} must be > 0." | ||
| ) | ||
| # Raise the potential error here if applicable, with feedback to user | ||
| # Else a normal error exit might occer which will not give feedback to the user | ||
| potential_error_to_error(self._error_type_check) |
There was a problem hiding this comment.
Can you maybe generalise this part of the code as it is almost similar as in the enabled part? E.g. make small method, named "attribute_value_not_zero" or something and call it both here and in in line 388-397
There was a problem hiding this comment.
Done. A function was added: _validate_attribute_value_not_zero
FJanssen-TNO
left a comment
FJanssen-TNO
left a comment
There was a problem hiding this comment.
The profile & range constraints indeed make it a bit more complicating, but I do think you found a proper way around it.
See a few generic comments from me below, besides that I think the structure of the new approach is okay.
| def __get_contraint_type_info(self, asset: Asset, constraint_type: esdl.Constraint) -> Union[ | ||
| List[int], int | ||
| ]: | ||
| """ | ||
| Get the contraint type info, if it exists, at an asset. | ||
|
|
||
| Arg: | ||
| asset: mesido common asset with all attributes | ||
| constraint type: the type of contraint specified (e.g. esdl.RangedConstraint, | ||
| esdl.ProfileConstraint) | ||
|
|
||
| Returns: | ||
| - Index of where the specific constraint is located in all the constraints specified | ||
| - Number of constraints of specific type that exists | ||
| ------- | ||
|
|
||
| """ | ||
| idx_of_range_constraints = [ | ||
| ii | ||
| for ii, xi in enumerate(asset.attributes["constraint"]) | ||
| if isinstance(xi, constraint_type) | ||
| ] # in the future me might want to cater for more than 1 range contraint | ||
| return idx_of_range_constraints, len(idx_of_range_constraints) | ||
|
|
There was a problem hiding this comment.
The exact same method was also created in profile_parser.py.
Maybe it can be made non-private method, in for example esdl_additional_vars_mixin.py or esdl_heat_model.py such that it can be called in both sections and we don't have code duplication.
| idx_of_range_constraints = [ | ||
| ii | ||
| for ii, xi in enumerate(asset.attributes["constraint"]) | ||
| if isinstance(xi, constraint_type) | ||
| ] # in the future me might want to cater for more than 1 range contraint |
There was a problem hiding this comment.
Why do we make a list of the indices of this type of constraint and why don't we just provide a list of the constraints are of this type?
| idx_of_range_constraints = [ | ||
| ii | ||
| for ii, xi in enumerate(asset.attributes["constraint"]) | ||
| if isinstance(xi, esdl.RangedConstraint) | ||
| ] # in the future me might want to cater for more than 1 range contraint | ||
| len_idx_of_range_constraints = len(idx_of_range_constraints) |
There was a problem hiding this comment.
This could aslo be replaced by __get_contraint_type_info()
3 participants
Update the esdl input variable used in MESIDO to limit the max size of an asset. Details in https://365tno.sharepoint.com/:x:/r/teams/P060.55186/TeamDocuments/Team/Work/Backend/Optimiser/ESDL%20compatibility%2020260121.xlsx?d=wbaacaf23f58f458091a47213eb3bedcd&csf=1&web=1&e=9LCNA7