diff --git a/antarest/study/storage/variantstudy/model/command/create_binding_constraint.py b/antarest/study/storage/variantstudy/model/command/create_binding_constraint.py index 495b48e557..75fe563911 100644 --- a/antarest/study/storage/variantstudy/model/command/create_binding_constraint.py +++ b/antarest/study/storage/variantstudy/model/command/create_binding_constraint.py @@ -84,7 +84,7 @@ class BindingConstraintProperties870(BindingConstraintProperties): group: t.Optional[str] = None -class BindingConstraintMatrices(BaseModel, extra=Extra.forbid): +class BindingConstraintMatrices(BaseModel, extra=Extra.forbid, allow_population_by_field_name=True): """ Class used to store the matrices of a binding constraint. """ @@ -96,14 +96,17 @@ class BindingConstraintMatrices(BaseModel, extra=Extra.forbid): less_term_matrix: t.Optional[t.Union[MatrixType, str]] = Field( None, description="less term matrix for v8.7+ studies", + alias="lessTermMatrix", ) greater_term_matrix: t.Optional[t.Union[MatrixType, str]] = Field( None, description="greater term matrix for v8.7+ studies", + alias="greaterTermMatrix", ) equal_term_matrix: t.Optional[t.Union[MatrixType, str]] = Field( None, description="equal term matrix for v8.7+ studies", + alias="equalTermMatrix", ) @root_validator(pre=True) diff --git a/antarest/study/storage/variantstudy/model/command/remove_area.py b/antarest/study/storage/variantstudy/model/command/remove_area.py index a93c3bcd84..8703137a5a 100644 --- a/antarest/study/storage/variantstudy/model/command/remove_area.py +++ b/antarest/study/storage/variantstudy/model/command/remove_area.py @@ -93,6 +93,7 @@ def _remove_area_from_binding_constraints(self, study_data: FileStudy) -> None: Instead, we decide to remove the binding constraints that are related to the area. """ + # See also `RemoveArea` # noinspection SpellCheckingInspection url = ["input", "bindingconstraints", "bindingconstraints"] binding_constraints = study_data.tree.get(url) @@ -100,6 +101,7 @@ def _remove_area_from_binding_constraints(self, study_data: FileStudy) -> None: # Collect the binding constraints that are related to the area to remove # by searching the terms that contain the ID of the area. bc_to_remove = {} + lower_area_id = self.id.lower() for bc_index, bc in list(binding_constraints.items()): for key in bc: # Term IDs are in the form `area1%area2` or `area.cluster` @@ -110,7 +112,8 @@ def _remove_area_from_binding_constraints(self, study_data: FileStudy) -> None: else: # This key belongs to the set of properties, it isn't a term ID, so we skip it continue - if self.id.lower() in related_areas: + related_areas = [area.lower() for area in related_areas] + if lower_area_id in related_areas: bc_to_remove[bc_index] = binding_constraints.pop(bc_index) break diff --git a/antarest/study/storage/variantstudy/model/command/remove_cluster.py b/antarest/study/storage/variantstudy/model/command/remove_cluster.py index 095e62f526..fbefaea312 100644 --- a/antarest/study/storage/variantstudy/model/command/remove_cluster.py +++ b/antarest/study/storage/variantstudy/model/command/remove_cluster.py @@ -134,26 +134,50 @@ def _create_diff(self, other: "ICommand") -> t.List["ICommand"]: def get_inner_matrices(self) -> t.List[str]: return [] - # noinspection SpellCheckingInspection def _remove_cluster_from_binding_constraints(self, study_data: FileStudy) -> None: - config = study_data.tree.get(["input", "bindingconstraints", "bindingconstraints"]) - - # Binding constraints IDs to remove - ids_to_remove = set() - - # Cluster IDs are stored in lower case in the binding contraints configuration file. - cluster_id = self.cluster_id.lower() - for bc_id, bc_props in config.items(): - if f"{self.area_id}.{cluster_id}" in bc_props.keys(): - ids_to_remove.add(bc_id) - - for bc_id in ids_to_remove: - study_data.tree.delete(["input", "bindingconstraints", config[bc_id]["id"]]) - bc = next(iter([bind for bind in study_data.config.bindings if bind.id == config[bc_id]["id"]])) - study_data.config.bindings.remove(bc) - del config[bc_id] - - study_data.tree.save( - config, - ["input", "bindingconstraints", "bindingconstraints"], - ) + """ + Remove the binding constraints that are related to the thermal cluster. + + Notes: + A binding constraint has properties, a list of terms (which form a linear equation) and + a right-hand side (which is the matrix of the binding constraint). + The terms are of the form `area1%area2` or `area.cluster` where `area` is the ID of the area + and `cluster` is the ID of the cluster. + + When a thermal cluster is removed, it has an impact on the terms of the binding constraints. + At first, we could decide to remove the terms that are related to the area. + However, this would lead to a linear equation that is not valid anymore. + + Instead, we decide to remove the binding constraints that are related to the cluster. + """ + # See also `RemoveCluster` + # noinspection SpellCheckingInspection + url = ["input", "bindingconstraints", "bindingconstraints"] + binding_constraints = study_data.tree.get(url) + + # Collect the binding constraints that are related to the area to remove + # by searching the terms that contain the ID of the area. + bc_to_remove = {} + lower_area_id = self.area_id.lower() + lower_cluster_id = self.cluster_id.lower() + for bc_index, bc in list(binding_constraints.items()): + for key in bc: + if "." not in key: + # This key identifies a link or belongs to the set of properties. + # It isn't a cluster ID, so we skip it. + continue + # Term IDs are in the form `area1%area2` or `area.cluster` + # noinspection PyTypeChecker + related_area_id, related_cluster_id = map(str.lower, key.split(".")) + if (lower_area_id, lower_cluster_id) == (related_area_id, related_cluster_id): + bc_to_remove[bc_index] = binding_constraints.pop(bc_index) + break + + matrix_suffixes = ["_lt", "_gt", "_eq"] if study_data.config.version >= 870 else [""] + + for bc_index, bc in bc_to_remove.items(): + for suffix in matrix_suffixes: + # noinspection SpellCheckingInspection + study_data.tree.delete(["input", "bindingconstraints", f"{bc['id']}{suffix}"]) + + study_data.tree.save(binding_constraints, url) diff --git a/tests/integration/study_data_blueprint/test_thermal.py b/tests/integration/study_data_blueprint/test_thermal.py index a44d7058ac..a164b58d8f 100644 --- a/tests/integration/study_data_blueprint/test_thermal.py +++ b/tests/integration/study_data_blueprint/test_thermal.py @@ -530,6 +530,29 @@ def test_lifecycle( # THERMAL CLUSTER DELETION # ============================= + # Here is a Binding Constraint that references the thermal cluster.: + bc_obj = { + "name": "Binding Constraint", + "enabled": True, + "time_step": "hourly", + "operator": "less", + "coeffs": {f"{area_id}.{fr_gas_conventional_id.lower()}": [2.0, 4]}, + "comments": "New API", + } + matrix = np.random.randint(0, 1000, size=(8784, 3)) + if version < 870: + bc_obj["values"] = matrix.tolist() + else: + bc_obj["lessTermMatrix"] = matrix.tolist() + + # noinspection SpellCheckingInspection + res = client.post( + f"/v1/studies/{study_id}/bindingconstraints", + json=bc_obj, + headers={"Authorization": f"Bearer {user_access_token}"}, + ) + assert res.status_code in {200, 201}, res.json() + # To delete a thermal cluster, we need to provide its ID. res = client.request( "DELETE", @@ -540,6 +563,15 @@ def test_lifecycle( assert res.status_code == 204, res.json() assert res.text in {"", "null"} # Old FastAPI versions return 'null'. + # When we delete a thermal cluster, we should also delete the binding constraints that reference it. + # noinspection SpellCheckingInspection + res = client.get( + f"/v1/studies/{study_id}/bindingconstraints", + headers={"Authorization": f"Bearer {user_access_token}"}, + ) + assert res.status_code == 200, res.json() + assert len(res.json()) == 0 + # If the thermal cluster list is empty, the deletion should be a no-op. res = client.request( "DELETE", diff --git a/tests/variantstudy/assets/empty_study_810.zip b/tests/variantstudy/assets/empty_study_810.zip new file mode 100644 index 0000000000..5d354af139 Binary files /dev/null and b/tests/variantstudy/assets/empty_study_810.zip differ diff --git a/tests/variantstudy/assets/empty_study_840.zip b/tests/variantstudy/assets/empty_study_840.zip new file mode 100644 index 0000000000..6954ff5841 Binary files /dev/null and b/tests/variantstudy/assets/empty_study_840.zip differ diff --git a/tests/variantstudy/assets/empty_study_870.zip b/tests/variantstudy/assets/empty_study_870.zip new file mode 100644 index 0000000000..7838152994 Binary files /dev/null and b/tests/variantstudy/assets/empty_study_870.zip differ diff --git a/tests/variantstudy/conftest.py b/tests/variantstudy/conftest.py index e963c83879..1bcbb13cb1 100644 --- a/tests/variantstudy/conftest.py +++ b/tests/variantstudy/conftest.py @@ -8,6 +8,12 @@ import numpy.typing as npt import pytest +from antarest.study.storage.study_upgrader import get_current_version + +if t.TYPE_CHECKING: + # noinspection PyPackageRequirements + from _pytest.fixtures import SubRequest + from antarest.matrixstore.model import MatrixDTO from antarest.matrixstore.service import MatrixService from antarest.matrixstore.uri_resolver_service import UriResolverService @@ -134,27 +140,32 @@ def command_factory_fixture(matrix_service: MatrixService) -> CommandFactory: @pytest.fixture(name="empty_study") -def empty_study_fixture(tmp_path: Path, matrix_service: MatrixService) -> FileStudy: +def empty_study_fixture(request: "SubRequest", tmp_path: Path, matrix_service: MatrixService) -> FileStudy: """ Fixture for creating an empty FileStudy object. Args: + request: pytest's request object. tmp_path: The temporary path for extracting the empty study. matrix_service: The MatrixService object. Returns: FileStudy: The empty FileStudy object. """ - empty_study_path: Path = ASSETS_DIR / "empty_study_720.zip" + zip_name = getattr(request, "param", "empty_study_720.zip") + empty_study_path: Path = ASSETS_DIR / zip_name empty_study_destination_path = tmp_path.joinpath("empty-study") with zipfile.ZipFile(empty_study_path, "r") as zip_empty_study: zip_empty_study.extractall(empty_study_destination_path) + # Detect the version of the study from `study.antares` file. + version = get_current_version(empty_study_destination_path) + config = FileStudyTreeConfig( study_path=empty_study_destination_path, path=empty_study_destination_path, study_id="", - version=720, + version=int(version), areas={}, sets={}, ) diff --git a/tests/variantstudy/model/command/test_manage_binding_constraints.py b/tests/variantstudy/model/command/test_manage_binding_constraints.py index bab8ff1681..aab13307b1 100644 --- a/tests/variantstudy/model/command/test_manage_binding_constraints.py +++ b/tests/variantstudy/model/command/test_manage_binding_constraints.py @@ -1,12 +1,16 @@ from unittest.mock import Mock import numpy as np +import pytest from antarest.study.storage.rawstudy.ini_reader import IniReader from antarest.study.storage.rawstudy.model.filesystem.config.binding_constraint import BindingConstraintFrequency from antarest.study.storage.rawstudy.model.filesystem.factory import FileStudy from antarest.study.storage.variantstudy.business.command_extractor import CommandExtractor from antarest.study.storage.variantstudy.business.command_reverter import CommandReverter +from antarest.study.storage.variantstudy.business.matrix_constants.binding_constraint.series_after_v87 import ( + default_bc_weekly_daily as default_bc_weekly_daily_870, +) from antarest.study.storage.variantstudy.business.matrix_constants.binding_constraint.series_before_v87 import ( default_bc_hourly, default_bc_weekly_daily, @@ -24,41 +28,18 @@ # noinspection SpellCheckingInspection -def test_manage_binding_constraint( - empty_study: FileStudy, - command_context: CommandContext, -): +@pytest.mark.parametrize("empty_study", ["empty_study_720.zip", "empty_study_870.zip"], indirect=True) +def test_manage_binding_constraint(empty_study: FileStudy, command_context: CommandContext): study_path = empty_study.config.study_path area1 = "area1" area2 = "area2" cluster = "cluster" - CreateArea.parse_obj( - { - "area_name": area1, - "command_context": command_context, - } - ).apply(empty_study) - CreateArea.parse_obj( - { - "area_name": area2, - "command_context": command_context, - } - ).apply(empty_study) - CreateLink.parse_obj( - { - "area1": area1, - "area2": area2, - "command_context": command_context, - } - ).apply(empty_study) + CreateArea.parse_obj({"area_name": area1, "command_context": command_context}).apply(empty_study) + CreateArea.parse_obj({"area_name": area2, "command_context": command_context}).apply(empty_study) + CreateLink.parse_obj({"area1": area1, "area2": area2, "command_context": command_context}).apply(empty_study) CreateCluster.parse_obj( - { - "area_id": area1, - "cluster_name": cluster, - "parameters": {}, - "command_context": command_context, - } + {"area_id": area1, "cluster_name": cluster, "parameters": {}, "command_context": command_context} ).apply(empty_study) bind1_cmd = CreateBindingConstraint( @@ -83,10 +64,18 @@ def test_manage_binding_constraint( res2 = bind2_cmd.apply(empty_study) assert res2.status - bc1_matrix_path = study_path / "input/bindingconstraints/bd 1.txt.link" - bc2_matrix_path = study_path / "input/bindingconstraints/bd 2.txt.link" - assert bc1_matrix_path.exists() - assert bc2_matrix_path.exists() + if empty_study.config.version < 870: + matrix_links = ["bd 1.txt.link", "bd 2.txt.link"] + else: + matrix_links = [ + # fmt: off + "bd 1_lt.txt.link", "bd 1_eq.txt.link", "bd 1_gt.txt.link", + "bd 2_lt.txt.link", "bd 2_eq.txt.link", "bd 2_gt.txt.link", + # fmt: on + ] + for matrix_link in matrix_links: + link_path = study_path / f"input/bindingconstraints/{matrix_link}" + assert link_path.exists(), f"Missing matrix link: {matrix_link!r}" cfg_path = study_path / "input/bindingconstraints/bindingconstraints.ini" bd_config = IniReader().read(cfg_path) @@ -108,14 +97,26 @@ def test_manage_binding_constraint( "type": "daily", } - weekly_values = default_bc_weekly_daily.tolist() + if empty_study.config.version < 870: + weekly_values = default_bc_weekly_daily.tolist() + values = weekly_values + less_term_matrix = None + greater_term_matrix = None + else: + weekly_values = default_bc_weekly_daily_870.tolist() + values = None + less_term_matrix = weekly_values + greater_term_matrix = weekly_values + bind_update = UpdateBindingConstraint( id="bd 1", enabled=False, time_step=BindingConstraintFrequency.WEEKLY, operator=BindingConstraintOperator.BOTH, coeffs={"area1%area2": [800, 30]}, - values=weekly_values, + values=values, + less_term_matrix=less_term_matrix, + greater_term_matrix=greater_term_matrix, command_context=command_context, ) res = bind_update.apply(empty_study) @@ -133,7 +134,16 @@ def test_manage_binding_constraint( remove_bind = RemoveBindingConstraint(id="bd 1", command_context=command_context) res3 = remove_bind.apply(empty_study) assert res3.status - assert not bc1_matrix_path.exists() + + for matrix_link in matrix_links: + link_path = study_path / f"input/bindingconstraints/{matrix_link}" + if matrix_link.startswith("bd 1"): + assert not link_path.exists(), f"Matrix link not removed: {matrix_link!r}" + elif matrix_link.startswith("bd 2"): + assert link_path.exists(), f"Matrix link removed: {matrix_link!r}" + else: + raise NotImplementedError(f"Unexpected matrix link: {matrix_link!r}") + bd_config = IniReader().read(cfg_path) assert len(bd_config) == 1 assert bd_config.get("0") == { diff --git a/tests/variantstudy/model/command/test_remove_area.py b/tests/variantstudy/model/command/test_remove_area.py index 3fb77082f2..118d45e0d8 100644 --- a/tests/variantstudy/model/command/test_remove_area.py +++ b/tests/variantstudy/model/command/test_remove_area.py @@ -3,7 +3,6 @@ from antarest.study.storage.rawstudy.model.filesystem.config.binding_constraint import BindingConstraintFrequency from antarest.study.storage.rawstudy.model.filesystem.config.model import transform_name_to_id from antarest.study.storage.rawstudy.model.filesystem.factory import FileStudy -from antarest.study.storage.study_upgrader import upgrade_study from antarest.study.storage.variantstudy.model.command.common import BindingConstraintOperator from antarest.study.storage.variantstudy.model.command.create_area import CreateArea from antarest.study.storage.variantstudy.model.command.create_binding_constraint import CreateBindingConstraint @@ -18,13 +17,8 @@ class TestRemoveArea: - @pytest.mark.parametrize("version", [810, 840]) - def test_apply( - self, - empty_study: FileStudy, - command_context: CommandContext, - version: int, - ): + @pytest.mark.parametrize("empty_study", ["empty_study_810.zip", "empty_study_840.zip"], indirect=True) + def test_apply(self, empty_study: FileStudy, command_context: CommandContext): # noinspection SpellCheckingInspection empty_study.tree.save( { @@ -72,10 +66,8 @@ def test_apply( ######################################################################################## - upgrade_study(empty_study.config.study_path, str(version)) - empty_study_cfg = empty_study.tree.get(depth=999) - if version >= 830: + if empty_study.config.version >= 830: empty_study_cfg["input"]["areas"][area_id]["adequacy_patch"] = { "adequacy-patch": {"adequacy-patch-mode": "outside"} } @@ -84,7 +76,6 @@ def test_apply( area_name2 = "Area2" area_id2 = transform_name_to_id(area_name2) - empty_study.config.version = version create_area_command: ICommand = CreateArea.parse_obj( { "area_name": area_name2, diff --git a/tests/variantstudy/model/command/test_remove_cluster.py b/tests/variantstudy/model/command/test_remove_cluster.py index 99333d811a..faae51f5c7 100644 --- a/tests/variantstudy/model/command/test_remove_cluster.py +++ b/tests/variantstudy/model/command/test_remove_cluster.py @@ -1,3 +1,5 @@ +import numpy as np +import pytest from checksumdir import dirhash from antarest.study.storage.rawstudy.model.filesystem.config.binding_constraint import BindingConstraintFrequency @@ -13,6 +15,7 @@ class TestRemoveCluster: + @pytest.mark.parametrize("empty_study", ["empty_study_720.zip", "empty_study_870.zip"], indirect=True) def test_apply(self, empty_study: FileStudy, command_context: CommandContext): area_name = "Area_name" area_id = transform_name_to_id(area_name) @@ -39,6 +42,15 @@ def test_apply(self, empty_study: FileStudy, command_context: CommandContext): modulation=[[0]], ).apply(empty_study) + # Binding constraint 2nd member: array of shape (8784, 3) + array = np.random.rand(8784, 3) * 1000 + if empty_study.config.version < 870: + values = array.tolist() + less_term_matrix = None + else: + values = None + less_term_matrix = array.tolist() + bind1_cmd = CreateBindingConstraint( name="BD 1", time_step=BindingConstraintFrequency.HOURLY, @@ -48,6 +60,8 @@ def test_apply(self, empty_study: FileStudy, command_context: CommandContext): }, comments="Hello", command_context=command_context, + values=values, + less_term_matrix=less_term_matrix, ) output = bind1_cmd.apply(study_data=empty_study) assert output.status