Skip to content
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

Bug: dimr_config for parallel run is incorrect #623

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
e297865
bug: update process on input for fmcomponent and add tests. (#562)
MRVermeulenDeltares Apr 17, 2024
689d592
bug: Resolve incorrect component use in test & rename variable. (#562)
MRVermeulenDeltares Apr 17, 2024
16b9b64
autoformat: isort & black
MRVermeulenDeltares Apr 17, 2024
5deeb66
bug: add raise valueerror on not int for process (#562)
MRVermeulenDeltares Apr 17, 2024
9683baa
autoformat: isort & black
MRVermeulenDeltares Apr 17, 2024
e374f81
bug: add validator instead of using the ctor (#562)
MRVermeulenDeltares Apr 18, 2024
e2dc8fe
autoformat: isort & black
MRVermeulenDeltares Apr 18, 2024
46792cd
bug: update that input of zero throws exception and input of one adds…
MRVermeulenDeltares Apr 25, 2024
07bb8e8
bug: implement newly discussed requirements and add tests. (#562)
MRVermeulenDeltares Apr 26, 2024
a03c692
autoformat: isort & black
MRVermeulenDeltares Apr 26, 2024
0549e32
bug: move tests to testclass.(#562)
MRVermeulenDeltares Apr 26, 2024
fbb37c0
autoformat: isort & black
MRVermeulenDeltares Apr 26, 2024
3879a20
bug: resolve codesmell.(#562)
MRVermeulenDeltares Apr 26, 2024
cc0d93e
bug: Refactor tests. (#562)
MRVermeulenDeltares Apr 26, 2024
9c99373
autoformat: isort & black
MRVermeulenDeltares Apr 26, 2024
61d50fa
bug: add multiple fmcomponent save test (#562)
MRVermeulenDeltares Apr 26, 2024
8f24b09
bug: Refactor dimr _save method (#562)
MRVermeulenDeltares Apr 26, 2024
a45cb2f
autoformat: isort & black
MRVermeulenDeltares Apr 26, 2024
a05fc37
bug: add support for reading process with semicolon, e.g. 0:4, assum…
MRVermeulenDeltares Apr 29, 2024
35adfcf
autoformat: isort & black
MRVermeulenDeltares Apr 29, 2024
e72c5ea
bug: resolve codesmells, make validation alike (#562)
MRVermeulenDeltares Apr 29, 2024
35f068a
autoformat: isort & black
MRVermeulenDeltares Apr 29, 2024
b0ab381
bug: update semicolon process and add tests (#562)
MRVermeulenDeltares Apr 29, 2024
935dd2b
bug: Resolve issue in which dimr writes None instead of leaving the p…
MRVermeulenDeltares Apr 29, 2024
996ebe5
autoformat: isort & black
MRVermeulenDeltares Apr 29, 2024
544bea7
bug: implement review suggestion moving changes to dict to _serialize…
MRVermeulenDeltares May 2, 2024
5d40188
bug: implement review suggestions (#562)
MRVermeulenDeltares May 2, 2024
cd48f61
bug: implement review suggestion moving string validation and process…
MRVermeulenDeltares May 2, 2024
37db323
autoformat: isort & black
MRVermeulenDeltares May 2, 2024
ac0086a
bug: implement review suggestions renaming private methods. (#562)
MRVermeulenDeltares May 2, 2024
e8c6ff6
bug: update method to private method (#562)
MRVermeulenDeltares May 2, 2024
b185a0b
bug: implement review suggestion add documentation for validation. (#…
MRVermeulenDeltares May 2, 2024
387bef5
autoformat: isort & black
MRVermeulenDeltares May 2, 2024
af0e19d
bug: implement review suggestion renaming values to process_value. (#…
MRVermeulenDeltares May 3, 2024
1da087c
bug: implement review suggestions for tests. (#562)
MRVermeulenDeltares May 3, 2024
dd46168
bug: move TestFmComponentProcess to another location and resolve smal…
MRVermeulenDeltares May 3, 2024
db9dd5f
autoformat: isort & black
MRVermeulenDeltares May 3, 2024
753a08c
Merge branch 'main' into bug/562-dimr_config-for-parallel-run-is-inco…
tim-vd-aardweg Jun 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
166 changes: 166 additions & 0 deletions hydrolib/core/dimr/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,43 @@ class FMComponent(Component):

library: Literal["dflowfm"] = "dflowfm"

@validator("process", pre=True)
def validate_process(cls, value, values: dict) -> Union[None, int]:
tim-vd-aardweg marked this conversation as resolved.
Show resolved Hide resolved
"""
Validation for the process Attribute.

args:
value : The value which is to be validated for process.
values : FMComponent used to retrieve the name of the component.

Returns:
int : The process as int, when given value is None, None is returned.

Raises:
ValueError : When value is set to 0 or negative.
ValueError : When value is not int or None.
"""
if value is None:
return value

if isinstance(value, int) and cls._is_valid_process_int(
value, values.get("name")
):
return value

raise ValueError(
f"In component '{values.get('name')}', the keyword process '{value}', is incorrect."
)

@classmethod
def _is_valid_process_int(cls, value: int, name: str) -> bool:
if value > 0:
return True

raise ValueError(
f"In component '{name}', the keyword process can not be 0 or negative, please specify value of 1 or greater."
)

@classmethod
def get_model(cls):
return FMModel
Expand Down Expand Up @@ -325,6 +362,65 @@ def _post_init_load(self) -> None:
except NotImplementedError:
pass

def _serialize(self, data: dict, save_settings: ModelSaveSettings) -> None:
dimr_as_dict = self._update_dimr_dictonary_with_adjusted_fmcomponent_values(
data
)
super()._serialize(dimr_as_dict, save_settings)

def _update_dimr_dictonary_with_adjusted_fmcomponent_values(
self, dimr_as_dict: Dict
):
fmcomponents = [
item for item in self.component if isinstance(item, FMComponent)
]

list_of_fmcomponents_as_dict = self._get_list_of_updated_fm_components(
fmcomponents
)
dimr_as_dict = self._update_dimr_dictionary(
dimr_as_dict, list_of_fmcomponents_as_dict
)
return dimr_as_dict

def _update_dimr_dictionary(
self, dimr_as_dict: Dict, list_of_fm_components_as_dict: List[Dict]
) -> Dict:
if len(list_of_fm_components_as_dict) > 0:
dimr_as_dict.update({"component": list_of_fm_components_as_dict})

return dimr_as_dict

def _get_list_of_updated_fm_components(
self, fmcomponents: List[FMComponent]
) -> List[Dict]:
list_of_fm_components_as_dict = []
for fmcomponent in fmcomponents:
if fmcomponent is None or fmcomponent.process is None:
continue

if fmcomponent.process == 1:
fmcomponent_as_dict = fmcomponent.dict()
fmcomponent_as_dict.pop("process", None)
else:
fmcomponent_process_value = " ".join(
str(i) for i in range(fmcomponent.process)
)
fmcomponent_as_dict = self._update_component_dictonary(
fmcomponent, fmcomponent_process_value
)

list_of_fm_components_as_dict.append(fmcomponent_as_dict)

return list_of_fm_components_as_dict

def _update_component_dictonary(
self, fmcomponent: FMComponent, fmcomponent_process_value: str
) -> Dict:
fmcomponent_as_dict = fmcomponent.dict()
fmcomponent_as_dict.update({"process": fmcomponent_process_value})
return fmcomponent_as_dict

@classmethod
def _ext(cls) -> str:
return ".xml"
Expand All @@ -342,3 +438,73 @@ def _get_serializer(
@classmethod
def _get_parser(cls) -> Callable:
return DIMRParser.parse

@classmethod
def _parse(cls, path: Path) -> Dict:
data = super()._parse(path)
return cls._update_component(data)

@classmethod
def _update_component(cls, data: Dict) -> Dict:
component = data.get("component", None)

if not isinstance(component, Dict):
return data

process_value = component.get("process", None)

if not isinstance(process_value, str):
return data

if cls._is_valid_process_string(process_value):
value_as_int = cls._parse_process(process_value)
component.update({"process": value_as_int})
data.update({"component": component})

return data

@classmethod
def _parse_process(cls, process_value: str) -> int:
if ":" in process_value:
semicolon_split_values = process_value.split(":")
start_value = int(semicolon_split_values[0])
end_value = int(semicolon_split_values[-1])
return end_value - start_value + 1

return len(process_value.split())

@classmethod
def _is_valid_process_string(cls, process_value: str) -> bool:
if ":" in process_value:
return cls._is_valid_process_with_semicolon_string(process_value)

return cls._is_valid_process_list_string(process_value)

@classmethod
def _is_valid_process_with_semicolon_string(cls, process_value: str) -> bool:
semicolon_split_values = process_value.split(":")

if len(semicolon_split_values) != 2:
return False

last_value: str = semicolon_split_values[-1]
if last_value.isdigit():
return True

return False

@classmethod
def _is_valid_process_list_string(cls, process_value: str) -> bool:
split_values = process_value.split()

if len(split_values) < 1:
return False

if split_values[0] != "0":
return False

for value in split_values:
if not value.isdigit():
return False

return True
106 changes: 105 additions & 1 deletion tests/dimr/test_dimr.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from hydrolib.core import __version__
from hydrolib.core.basemodel import ModelSaveSettings, SerializerConfig
from hydrolib.core.dimr.models import DIMR
from hydrolib.core.dimr.models import FMComponent
from hydrolib.core.dimr.parser import DIMRParser
from hydrolib.core.dimr.serializer import DIMRSerializer
from tests.utils import (
Expand Down Expand Up @@ -145,3 +145,107 @@ def test_serialize_float_are_formatted():

assert file_path.is_file()
assert_files_equal(file_path, reference_file)


class TestFmComponentProcess:
@pytest.mark.parametrize(
"input_process",
[
pytest.param(None),
pytest.param(1),
pytest.param(2),
pytest.param(3),
pytest.param(4),
pytest.param(5),
],
)
def test_fmcomponent_process_after_init(self, input_process: int):
component = FMComponent(
name="test",
workingDir=".",
inputfile="test.mdu",
process=input_process,
mpiCommunicator="DFM_COMM_DFMWORLD",
)
assert component.process == input_process

@pytest.mark.parametrize(
"input_process",
[
pytest.param(None),
pytest.param(1),
pytest.param(2),
pytest.param(3),
pytest.param(4),
pytest.param(5),
],
)
def test_fmcomponent_process_after_setting(self, input_process: int):
component = FMComponent(
name="test",
workingDir=".",
inputfile="test.mdu",
mpiCommunicator="DFM_COMM_DFMWORLD",
)
component.process = input_process
assert component.process == input_process

def test_fmcomponent_without_process_after_init(self):
expected_process_format = None
component = FMComponent(
name="test",
workingDir=".",
inputfile="test.mdu",
mpiCommunicator="DFM_COMM_DFMWORLD",
)
assert component.process == expected_process_format

@pytest.mark.parametrize(
"input_process",
[
pytest.param("lalala"),
pytest.param("123"),
pytest.param(3.5),
],
)
def test_fmcomponent_process_after_init_with_invalid_type_input_raises_value_error(
self,
input_process,
):
with pytest.raises(ValueError) as error:
FMComponent(
name="test",
workingDir=".",
inputfile="test.mdu",
process=input_process,
mpiCommunicator="DFM_COMM_DFMWORLD",
)

expected_message = (
f"In component 'test', the keyword process '{input_process}', is incorrect."
)
assert expected_message in str(error.value)

@pytest.mark.parametrize(
"input_process",
[
pytest.param(0),
pytest.param(-1),
pytest.param(-3),
],
)
def test_fmcomponent_process_after_init_with_input_process_zero_or_negative_raises_value_error(
self,
input_process: int,
):
with pytest.raises(ValueError) as error:
FMComponent(
name="test",
workingDir=".",
inputfile="test.mdu",
process=input_process,
mpiCommunicator="DFM_COMM_DFMWORLD",
)

expected_message = "In component 'test', the keyword process can not be 0 or negative, please specify value of 1 or greater."
assert expected_message in str(error.value)