From 1395e9c884415964cf7b58607f864ca5e5170c0c Mon Sep 17 00:00:00 2001 From: Laurent LAPORTE Date: Fri, 22 Mar 2024 10:00:45 +0100 Subject: [PATCH] fix(bc): remove group validation from BC creation --- .../business/binding_constraint_management.py | 68 ++++-- .../test_binding_constraints.py | 196 +++++++++--------- 2 files changed, 153 insertions(+), 111 deletions(-) diff --git a/antarest/study/business/binding_constraint_management.py b/antarest/study/business/binding_constraint_management.py index 0434c0e738..241d0cf98a 100644 --- a/antarest/study/business/binding_constraint_management.py +++ b/antarest/study/business/binding_constraint_management.py @@ -1,6 +1,7 @@ from typing import Any, Dict, List, Optional, Union -from pydantic import BaseModel, validator +import numpy as np +from pydantic import BaseModel, validator, root_validator from antarest.core.exceptions import ( BindingConstraintNotFoundError, @@ -228,6 +229,57 @@ class BindingConstraintCreation(BindingConstraintMatrices, BindingConstraintProp name: str coeffs: Dict[str, List[float]] + # Ajout d'un root validator pour valider les dimensions des matrices + @root_validator(pre=True) + def check_matrices_dimensions(cls, values: Dict[str, Any]) -> Dict[str, Any]: + # The dimensions of the matrices depend on the frequency and the version of the study. + if values.get("time_step") is None: + return values + _time_step = BindingConstraintFrequency(values["time_step"]) + + # Matrix shapes for binding constraints are different from usual shapes, + # because we need to take leap years into account, which contains 366 days and 8784 hours. + # Also, we use the same matrices for "weekly" and "daily" frequencies, + # because the solver calculates the weekly matrix from the daily matrix. + # See https://github.com/AntaresSimulatorTeam/AntaREST/issues/1843 + expected_rows = { + BindingConstraintFrequency.HOURLY: 8784, + BindingConstraintFrequency.DAILY: 366, + BindingConstraintFrequency.WEEKLY: 366, + }[_time_step] + + # Collect the matrix shapes + matrix_shapes = {} + for _field_name in ["values", "less_term_matrix", "equal_term_matrix", "greater_term_matrix"]: + if _matrix := values.get(_field_name): + _array = np.array(_matrix) + # We only store the shape if the array is not empty + if _array.size != 0: + matrix_shapes[_field_name] = _array.shape + + # We don't know the exact version of the study here, but we can rely on the matrix field names. + if not matrix_shapes: + return values + elif "values" in matrix_shapes: + expected_cols = 3 + else: + # pick the first matrix column as the expected column + expected_cols = next(iter(matrix_shapes.values()))[1] + + if all(shape == (expected_rows, expected_cols) for shape in matrix_shapes.values()): + return values + + # Prepare a clear error message + _field_names = ", ".join(f"'{n}'" for n in matrix_shapes) + if len(matrix_shapes) == 1: + err_msg = f"Matrix {_field_names} must have the shape ({expected_rows}, {expected_cols})" + else: + _shapes = list({(expected_rows, s[1]) for s in matrix_shapes.values()}) + _shapes_msg = ", ".join(f"{s}" for s in _shapes[:-1]) + " or " + f"{_shapes[-1]}" + err_msg = f"Matrices {_field_names} must have the same shape: {_shapes_msg}" + + raise ValueError(err_msg) + class BindingConstraintConfig(BindingConstraintProperties): id: str @@ -379,16 +431,6 @@ def create_binding_constraint( check_attributes_coherence(data, version) - file_study = self.storage_service.get_storage(study).get_raw(study) - if version >= 870: - data.group = data.group or "default" - matrix_terms_list = { - "eq": data.equal_term_matrix, - "lt": data.less_term_matrix, - "gt": data.greater_term_matrix, - } - check_matrices_coherence(file_study, data.group, bc_id, matrix_terms_list) - args = { "name": data.name, "enabled": data.enabled, @@ -404,7 +446,7 @@ def create_binding_constraint( "comments": data.comments or "", } if version >= 870: - args["group"] = data.group + args["group"] = data.group or "default" command = CreateBindingConstraint( **args, command_context=self.storage_service.variant_study_service.command_factory.command_context @@ -413,6 +455,8 @@ def create_binding_constraint( # Validates the matrices. Needed when the study is a variant because we only append the command to the list if isinstance(study, VariantStudy): command.validates_and_fills_matrices(specific_matrices=None, version=version, create=True) + + file_study = self.storage_service.get_storage(study).get_raw(study) execute_or_add_commands(study, file_study, [command], self.storage_service) # Processes the constraints to add them inside the endpoint response. diff --git a/tests/integration/study_data_blueprint/test_binding_constraints.py b/tests/integration/study_data_blueprint/test_binding_constraints.py index d9899e6b85..f1c9b80ad1 100644 --- a/tests/integration/study_data_blueprint/test_binding_constraints.py +++ b/tests/integration/study_data_blueprint/test_binding_constraints.py @@ -547,11 +547,12 @@ def test_lifecycle__nominal(self, client: TestClient, user_access_token: str, st json=wrong_request_args, headers=user_headers, ) - assert res.status_code == 500 + assert res.status_code == 422, res.json() exception = res.json()["exception"] description = res.json()["description"] - assert exception == "ValueError" if study_type == "variant" else "CommandApplicationError" - assert f"Invalid matrix shape {wrong_matrix.shape}, expected (366, 3)" in description + assert exception == "RequestValidationError" + assert "'values'" in description + assert "(366, 3)" in description # Delete a fake binding constraint res = client.delete(f"/v1/studies/{study_id}/bindingconstraints/fake_bc", headers=user_headers) @@ -636,11 +637,10 @@ def test_for_version_870(self, client: TestClient, admin_access_token: str, stud # Creation of bc with a matrix bc_id_w_matrix = "binding_constraint_3" - matrix_lt = np.ones((8784, 3)) - matrix_lt_to_list = matrix_lt.tolist() + matrix_lt3 = np.ones((8784, 3)) res = client.post( f"/v1/studies/{study_id}/bindingconstraints", - json={"name": bc_id_w_matrix, "less_term_matrix": matrix_lt_to_list, **args}, + json={"name": bc_id_w_matrix, "less_term_matrix": matrix_lt3.tolist(), **args}, headers=admin_headers, ) assert res.status_code in {200, 201}, res.json() @@ -663,9 +663,9 @@ def test_for_version_870(self, client: TestClient, admin_access_token: str, stud assert res.status_code == 200 data = res.json()["data"] if term == "lt": - assert data == matrix_lt_to_list + assert data == matrix_lt3.tolist() else: - assert data == np.zeros((matrix_lt.shape[0], 1)).tolist() + assert data == np.zeros((matrix_lt3.shape[0], 1)).tolist() # ============================= # UPDATE @@ -684,7 +684,7 @@ def test_for_version_870(self, client: TestClient, admin_access_token: str, stud # Update matrix_term res = client.put( f"/v1/studies/{study_id}/bindingconstraints/{bc_id_w_matrix}", - json={"greater_term_matrix": matrix_lt_to_list}, + json={"greater_term_matrix": matrix_lt3.tolist()}, headers=admin_headers, ) assert res.status_code == 200, res.json() @@ -695,7 +695,7 @@ def test_for_version_870(self, client: TestClient, admin_access_token: str, stud headers=admin_headers, ) assert res.status_code == 200 - assert res.json()["data"] == matrix_lt_to_list + assert res.json()["data"] == matrix_lt3.tolist() # The user changed the time_step to daily instead of hourly. # We must check that the matrices have been updated. @@ -707,11 +707,12 @@ def test_for_version_870(self, client: TestClient, admin_access_token: str, stud assert res.status_code == 200, res.json() if study_type == "variant": - # Check the last command is a change time_step + # Check the last command is a change on `time_step` field only res = client.get(f"/v1/studies/{study_id}/commands", headers=admin_headers) commands = res.json() command_args = commands[-1]["args"] assert command_args["time_step"] == "daily" + assert "values" not in command_args assert ( command_args["less_term_matrix"] == command_args["greater_term_matrix"] @@ -720,7 +721,7 @@ def test_for_version_870(self, client: TestClient, admin_access_token: str, stud ) # Check that the matrices are daily/weekly matrices - expected_matrix = np.zeros((366, 1)).tolist() + expected_matrix = np.zeros((366, 1)) for term_alias in ["lt", "gt", "eq"]: res = client.get( f"/v1/studies/{study_id}/raw", @@ -732,7 +733,7 @@ def test_for_version_870(self, client: TestClient, admin_access_token: str, stud headers=admin_headers, ) assert res.status_code == 200 - assert res.json()["data"] == expected_matrix + assert res.json()["data"] == expected_matrix.tolist() # ============================= # DELETE @@ -779,149 +780,146 @@ def test_for_version_870(self, client: TestClient, admin_access_token: str, stud # Creation with 2 matrices with different columns size bc_id_with_wrong_matrix = "binding_constraint_with_wrong_matrix" - matrix_lt = np.ones((8784, 3)) - matrix_gt = np.ones((8784, 2)) - matrix_gt_to_list = matrix_gt.tolist() - matrix_lt_to_list = matrix_lt.tolist() + matrix_lt3 = np.ones((8784, 3)) + matrix_gt2 = np.ones((8784, 2)) # Wrong number of columns res = client.post( f"/v1/studies/{study_id}/bindingconstraints", json={ "name": bc_id_with_wrong_matrix, - "less_term_matrix": matrix_lt_to_list, - "greater_term_matrix": matrix_gt_to_list, + "less_term_matrix": matrix_lt3.tolist(), + "greater_term_matrix": matrix_gt2.tolist(), **args, }, headers=admin_headers, ) - assert res.status_code == 422 - assert res.json()["exception"] == "IncoherenceBetweenMatricesLength" - assert ( - res.json()["description"] - == "The matrices of binding_constraint_with_wrong_matrix must have the same number of columns, currently {2, 3}" - ) + assert res.status_code == 422, res.json() + exception = res.json()["exception"] + description = res.json()["description"] + assert exception == "RequestValidationError" + assert "'less_term_matrix'" in description + assert "'greater_term_matrix'" in description + assert "(8784, 3)" in description + assert "(8784, 2)" in description # - # Creation of 2 bc inside the same group with different columns size - # first_bc: 3 cols, group1 - # second_bc: 4 cols, group1 -> Should fail + # Creation of 2 BC inside the same group with different columns size + # "First BC": 3 cols, "Group 1" -> OK + # "Second BC": 4 cols, "Group 1" -> OK, but should fail in group validation # - first_bc = "binding_constraint_validation" - matrix_lt = np.ones((8784, 3)) - matrix_lt_to_list = matrix_lt.tolist() + matrix_lt3 = np.ones((8784, 3)) res = client.post( f"/v1/studies/{study_id}/bindingconstraints", - json={"name": first_bc, "less_term_matrix": matrix_lt_to_list, "group": "group1", **args}, + json={ + "name": "First BC", + "less_term_matrix": matrix_lt3.tolist(), + "group": "Group 1", + **args, + }, headers=admin_headers, ) assert res.status_code in {200, 201}, res.json() - matrix_gt = np.ones((8784, 4)) - matrix_gt_to_list = matrix_gt.tolist() + matrix_gt4 = np.ones((8784, 4)) # Wrong number of columns res = client.post( f"/v1/studies/{study_id}/bindingconstraints", - json={"name": "other_bc", "greater_term_matrix": matrix_gt_to_list, "group": "group1", **args}, + json={ + "name": "Second BC", + "greater_term_matrix": matrix_gt4.tolist(), + "group": "group 1", # Same group, but different case + **args, + }, headers=admin_headers, ) - assert res.status_code == 422 - assert res.json()["exception"] == "IncoherenceBetweenMatricesLength" - assert res.json()["description"] == "The matrices of the group group1 do not have the same number of columns" + assert res.status_code in {200, 201}, res.json() + second_bc_id = res.json()["id"] + + # todo: validate the BC group "Group 1" + # res = client.get(f"/v1/studies/{study_id}/bindingconstraints/Group 1/validate", headers=admin_headers) + # assert res.status_code == 422 + # assert res.json()["exception"] == "IncoherenceBetweenMatricesLength" + # assert res.json()["description"] == "Mismatched column count in 'Group 1'". + + # So, we correct the shape of the matrix of the Second BC + res = client.put( + f"/v1/studies/{study_id}/bindingconstraints/{second_bc_id}", + json={"greater_term_matrix": matrix_lt3.tolist()}, + headers=admin_headers, + ) + assert res.status_code in {200, 201}, res.json() # # Updating the group of a bc creates different columns size inside the same group - # first_bc: 3 cols, group 1 - # second_bc: 4 cols, group2 -> OK - # second_bc group changes to group1 -> Fails validation + # first_bc: 3 cols, "Group 1" -> OK + # third_bd: 4 cols, "Group 2" -> OK + # third_bd group changes to group1 -> Fails validation # - second_bc = "binding_constraint_validation_2" - matrix_lt = np.ones((8784, 4)) - matrix_lt_to_list = matrix_lt.tolist() + matrix_lt4 = np.ones((8784, 4)) res = client.post( f"/v1/studies/{study_id}/bindingconstraints", - json={"name": second_bc, "less_term_matrix": matrix_lt_to_list, "group": "group2", **args}, + json={ + "name": "Third BC", + "less_term_matrix": matrix_lt4.tolist(), + "group": "Group 2", + **args, + }, headers=admin_headers, ) assert res.status_code in {200, 201}, res.json() + third_bd_id = res.json()["id"] res = client.put( - f"v1/studies/{study_id}/bindingconstraints/{second_bc}", - json={"group": "group1"}, + f"v1/studies/{study_id}/bindingconstraints/{third_bd_id}", + json={"group": "Group 1"}, headers=admin_headers, ) # This should succeed but cause the validation endpoint to fail. assert res.status_code in {200, 201}, res.json() - res = client.get(f"/v1/studies/{study_id}/bindingconstraints/{second_bc}/validate", headers=admin_headers) - assert res.status_code == 422 - assert res.json()["exception"] == "IncoherenceBetweenMatricesLength" - assert res.json()["description"] == "The matrices of the group group1 do not have the same number of columns" - - # - # Update causes different matrices size inside the same bc - # second_bc: 1st matrix has 4 cols and others 1 -> OK - # second_bc: 1st matrix has 4 cols and 2nd matrix has 3 cols -> Fails validation - # + # todo: validate the BC group "Group 1" + # res = client.get(f"/v1/studies/{study_id}/bindingconstraints/Group 1/validate", headers=admin_headers) + # assert res.status_code == 422 + # assert res.json()["exception"] == "IncoherenceBetweenMatricesLength" + # assert res.json()["description"] == "Mismatched column count in 'Group 1'". + # So, we correct the shape of the matrix of the Second BC res = client.put( - f"v1/studies/{study_id}/bindingconstraints/{second_bc}", json={"group": "group2"}, headers=admin_headers - ) - assert res.status_code in {200, 201}, res.json() - # For the moment the bc is valid - res = client.get(f"/v1/studies/{study_id}/bindingconstraints/{second_bc}/validate", headers=admin_headers) - assert res.status_code in {200, 201}, res.json() - - matrix_lt_3 = np.ones((8784, 3)) - matrix_lt_3_to_list = matrix_lt_3.tolist() - res = client.put( - f"v1/studies/{study_id}/bindingconstraints/{second_bc}", - json={"greater_term_matrix": matrix_lt_3_to_list}, + f"/v1/studies/{study_id}/bindingconstraints/{third_bd_id}", + json={"greater_term_matrix": matrix_lt3.tolist()}, headers=admin_headers, ) - # This should succeed but cause the validation endpoint to fail. assert res.status_code in {200, 201}, res.json() - res = client.get(f"/v1/studies/{study_id}/bindingconstraints/{second_bc}/validate", headers=admin_headers) - assert res.status_code == 422 - assert res.json()["exception"] == "IncoherenceBetweenMatricesLength" - assert ( - res.json()["description"] - == "The matrices of binding_constraint_validation_2 must have the same number of columns, currently {3, 4}" - ) - # - # Updating a matrix causes different matrices size inside the same group - # first_bc: 3 cols, group1 - # second_bc: 3 cols, group1 -> OK - # second_bc: update 2 matrices with 4 cols, group1 -> Fails validation + # Update causes different matrices size inside the same bc + # second_bc: 1st matrix has 4 cols and others 1 -> OK + # second_bc: 1st matrix has 4 cols and 2nd matrix has 3 cols -> Fails validation # res = client.put( - f"v1/studies/{study_id}/bindingconstraints/{second_bc}", - json={"less_term_matrix": matrix_lt_3_to_list}, + f"v1/studies/{study_id}/bindingconstraints/{second_bc_id}", + json={"group": "Group 2"}, headers=admin_headers, ) assert res.status_code in {200, 201}, res.json() - # For the moment the bc is valid - res = client.get(f"/v1/studies/{study_id}/bindingconstraints/{second_bc}/validate", headers=admin_headers) - assert res.status_code in {200, 201}, res.json() + # todo: validate the "Group 2" + # # For the moment the bc is valid + # res = client.get(f"/v1/studies/{study_id}/bindingconstraints/Group 2/validate", headers=admin_headers) + # assert res.status_code in {200, 201}, res.json() res = client.put( - f"v1/studies/{study_id}/bindingconstraints/{second_bc}", - json={"group": "group1"}, - headers=admin_headers, - ) - assert res.status_code in {200, 201}, res.json() - res = client.put( - f"v1/studies/{study_id}/bindingconstraints/{second_bc}", - json={"less_term_matrix": matrix_lt_to_list, "greater_term_matrix": matrix_lt_to_list}, + f"v1/studies/{study_id}/bindingconstraints/{second_bc_id}", + json={"greater_term_matrix": matrix_lt3.tolist()}, headers=admin_headers, ) # This should succeed but cause the validation endpoint to fail. assert res.status_code in {200, 201}, res.json() - res = client.get(f"/v1/studies/{study_id}/bindingconstraints/{second_bc}/validate", headers=admin_headers) - assert res.status_code == 422 - assert res.json()["exception"] == "IncoherenceBetweenMatricesLength" - assert res.json()["description"] == "The matrices of the group group1 do not have the same number of columns" + # For the moment the "Group 2" is valid + # todo: validate the "Group 2" + # res = client.get(f"/v1/studies/{study_id}/bindingconstraints/Group 2/validate", headers=admin_headers) + # assert res.status_code == 422 + # assert res.json()["exception"] == "IncoherenceBetweenMatricesLength" + # assert res.json()["description"] == "Mismatched column count in 'Group 2'".