From bd508a08a52ab0868758a4a49616659962533424 Mon Sep 17 00:00:00 2001 From: Bart Schilperoort Date: Fri, 22 Nov 2024 14:25:04 +0100 Subject: [PATCH 1/7] Allow starting remotebmi containers --- src/ewatercycle/base/model.py | 4 ++- src/ewatercycle/container.py | 66 ++++++++++++++++++++++++++--------- 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/src/ewatercycle/base/model.py b/src/ewatercycle/base/model.py index 08cf5d5f..68f1445b 100644 --- a/src/ewatercycle/base/model.py +++ b/src/ewatercycle/base/model.py @@ -8,7 +8,7 @@ from contextlib import suppress from datetime import timezone from pathlib import Path -from typing import Annotated, Any, cast +from typing import Annotated, Any, Literal, cast import bmipy import numpy as np @@ -439,6 +439,7 @@ class ContainerizedModel(eWaterCycleModel): """ bmi_image: Annotated[ContainerImage, BeforeValidator(_parse_containerimage)] + protocol: Literal["grpc", "openapi"] = "grpc" # Create as empty list to allow models to append before bmi is made: _additional_input_dirs: list[str] = PrivateAttr([]) @@ -459,6 +460,7 @@ def _make_bmi_instance(self) -> OptionalDestBmi: return start_container( image=self.bmi_image, + protocol=self.protocol, work_dir=self._cfg_dir, input_dirs=self._additional_input_dirs, timeout=300, diff --git a/src/ewatercycle/container.py b/src/ewatercycle/container.py index 556694c0..0f296ae0 100644 --- a/src/ewatercycle/container.py +++ b/src/ewatercycle/container.py @@ -5,9 +5,10 @@ import re from collections.abc import Iterable, Sequence from pathlib import Path -from typing import Any, Protocol +from typing import Any, Literal, Protocol import numpy as np +import remotebmi from bmipy import Bmi from grpc import FutureTimeoutError from grpc4bmi.bmi_client_apptainer import BmiClientApptainer @@ -136,6 +137,7 @@ def start_container( delay=0, # TODO replace Any type with Bmi + BmiFromOrigin wrappers: Sequence[type[Any]] = (MemoizedBmi, OptionalDestBmi), + protocol: Literal["grpc", "openapi"] = "grpc" ) -> OptionalDestBmi: """Start container with model inside. @@ -152,6 +154,7 @@ def start_container( delay: Number of seconds to wait before connecting. wrappers: List of classes to wrap around the grcp4bmi object from container. Order is important. The first wrapper is the most inner wrapper. + protocol: Which protocol to use, grpc or openapi. Raises: ValueError: When unknown container technology is requested. @@ -184,7 +187,7 @@ def start_container( if engine == "docker": bmi = start_docker_container( - work_dir, image, input_dirs, image_port, timeout, delay + work_dir, image, input_dirs, image_port, timeout, delay, protocol, ) elif engine == "apptainer": bmi = start_apptainer_container( @@ -193,6 +196,7 @@ def start_container( input_dirs, timeout, delay, + protocol, ) else: msg = f"Unknown container technology: {CFG.container_engine}" @@ -209,6 +213,7 @@ def start_apptainer_container( input_dirs: Iterable[str] = (), timeout: int | None = None, delay: int = 0, + protocol: Literal["grpc", "openapi"] = "grpc", ) -> Bmi: """Start Apptainer container with model inside. @@ -220,6 +225,7 @@ def start_apptainer_container( input_dirs: Additional directories to mount inside container. timeout: Number of seconds to wait for grpc connection. delay: Number of seconds to wait before connecting. + protocol: Which protocol to use, grpc or openapi. .. _apptainer manual: https://apptainer.org/docs/user/latest/cli/apptainer_run.html @@ -234,13 +240,25 @@ def start_apptainer_container( image_fn = str(CFG.apptainer_dir / image_fn) try: - return BmiClientApptainer( - image=image_fn, - work_dir=str(work_dir), - input_dirs=input_dirs, - timeout=timeout, - delay=delay, - ) + if protocol == "grpc": + return BmiClientApptainer( + image=image_fn, + work_dir=str(work_dir), + input_dirs=input_dirs, + timeout=timeout, + delay=delay, + ) + if protocol == "openapi": + return remotebmi.BmiClientApptainer( + image=image_fn, + work_dir=str(work_dir), + input_dirs=input_dirs, + delay=delay, + ) + msg = f"Invalid protocol '{protocol}'!" + raise ValueError(msg) + + except FutureTimeoutError as exc: msg = ( "Couldn't spawn container within allocated time limit " @@ -258,6 +276,7 @@ def start_docker_container( image_port=55555, timeout=None, delay=0, + protocol: Literal["grpc", "openapi"] = "grpc", ): """Start Docker container with model inside. @@ -268,6 +287,7 @@ def start_docker_container( image_port: Docker port inside container where grpc4bmi server is running. timeout: Number of seconds to wait for grpc connection. delay: Number of seconds to wait before connecting. + protocol: Which protocol to use, grpc or openapi. Raises: TimeoutError: When model inside container did not start quickly enough. @@ -276,14 +296,26 @@ def start_docker_container( Bmi object which wraps the container. """ try: - return BmiClientDocker( - image=image.docker_url, - image_port=image_port, - work_dir=str(work_dir), - input_dirs=input_dirs, - timeout=timeout, - delay=delay, - ) + if protocol == "grpc": + return BmiClientDocker( + image=image.docker_url, + image_port=image_port, + work_dir=str(work_dir), + input_dirs=input_dirs, + timeout=timeout, + delay=delay, + ) + if protocol == "openapi": + return remotebmi.BmiClientDocker( + image=image.docker_url, + host="localhost", + image_port=50051, + work_dir=str(work_dir), + input_dirs=input_dirs, + delay=delay, + ) + msg = f"Invalid protocol '{protocol}'!" + raise ValueError(msg) except FutureTimeoutError as exc: # https://github.com/eWaterCycle/grpc4bmi/issues/95 # https://github.com/eWaterCycle/grpc4bmi/issues/100 From e2d03990c94fa476604133b0ce880d683a1c64bb Mon Sep 17 00:00:00 2001 From: Bart Schilperoort Date: Wed, 27 Nov 2024 11:46:15 +0100 Subject: [PATCH 2/7] Correct formatting --- src/ewatercycle/container.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/ewatercycle/container.py b/src/ewatercycle/container.py index 0f296ae0..8c3f3c2d 100644 --- a/src/ewatercycle/container.py +++ b/src/ewatercycle/container.py @@ -137,7 +137,7 @@ def start_container( delay=0, # TODO replace Any type with Bmi + BmiFromOrigin wrappers: Sequence[type[Any]] = (MemoizedBmi, OptionalDestBmi), - protocol: Literal["grpc", "openapi"] = "grpc" + protocol: Literal["grpc", "openapi"] = "grpc", ) -> OptionalDestBmi: """Start container with model inside. @@ -187,7 +187,13 @@ def start_container( if engine == "docker": bmi = start_docker_container( - work_dir, image, input_dirs, image_port, timeout, delay, protocol, + work_dir, + image, + input_dirs, + image_port, + timeout, + delay, + protocol, ) elif engine == "apptainer": bmi = start_apptainer_container( @@ -258,7 +264,6 @@ def start_apptainer_container( msg = f"Invalid protocol '{protocol}'!" raise ValueError(msg) - except FutureTimeoutError as exc: msg = ( "Couldn't spawn container within allocated time limit " From dfc10f8b8628bb59fd8c2740e9f065fbef2dd221 Mon Sep 17 00:00:00 2001 From: Bart Schilperoort Date: Wed, 27 Nov 2024 11:46:38 +0100 Subject: [PATCH 3/7] Ignore linter rule in notebooks --- pyproject.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 1ad3982a..0e230da2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -205,6 +205,8 @@ ignore = [ "E501", # Allow prints "T201", + # Allow shadowing builtins + "A004", ] [tool.ruff.lint.pydocstyle] From 522156f813b9f87306ce2a5a8a29c5f93dbcb4f3 Mon Sep 17 00:00:00 2001 From: Bart Schilperoort Date: Wed, 27 Nov 2024 11:48:41 +0100 Subject: [PATCH 4/7] Add remotebmi to dependencies --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 0e230da2..466059f8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -40,6 +40,7 @@ dependencies = [ # otherwise pip installation fails on esmpy "Fiona", "grpc4bmi>=0.4.0", + "remotebmi", "hydrostats", "matplotlib>=3.5.0", "numpy", From 5692b7b4c452dfdaff9bbe3ea113b2465d2ac2f8 Mon Sep 17 00:00:00 2001 From: Bart Schilperoort Date: Wed, 27 Nov 2024 11:49:32 +0100 Subject: [PATCH 5/7] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2320bb9a..9e12e79f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Formatted as described on [https://keepachangelog.com](https://keepachangelog.co ## Added +- support for [Remote BMI](https://github.com/eWaterCycle/remotebmi), an OpenAPI based alternative for grpc4bmi ([#467](https://github.com/eWaterCycle/ewatercycle/pull/467)). - `.get_shape_area()` utility method to the ewatercycle Forcing objects. This returns the area of the shapefile in square meters, useful for converting the results of lumped models (e.g., from mm/day to m3/s) ([#464](https://github.com/eWaterCycle/ewatercycle/issues/464)). - `.plot_shape()` utility method to the ewatercycle Forcing objects. This allows plotting the shapefile in a single-line of code, or adds the shapefile to an existing plot ([#464](https://github.com/eWaterCycle/ewatercycle/issues/464)). From cd25e1e8f1ac6ad76ff64dc15ea448994b8bf909 Mon Sep 17 00:00:00 2001 From: Bart Schilperoort Date: Wed, 27 Nov 2024 11:57:13 +0100 Subject: [PATCH 6/7] Update tests to reflect changes --- tests/src/base/test_model.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/src/base/test_model.py b/tests/src/base/test_model.py index d743de70..978fb01f 100644 --- a/tests/src/base/test_model.py +++ b/tests/src/base/test_model.py @@ -312,6 +312,7 @@ def test_setup(self, mocked_start_container, tmp_path: Path): mocked_start_container.assert_called_once_with( image="ewatercycle/ewatercycle_dummy:latest", + protocol="grpc", work_dir=tmp_path, input_dirs=[], timeout=300, @@ -345,6 +346,7 @@ def test_setup_with_additional_input_dirs( mocked_start_container.assert_called_once_with( image="ewatercycle/ewatercycle_dummy:latest", + protocol="grpc", work_dir=tmp_path, input_dirs=[ str(parameter_set_dir), From f66a4a07ab80125591a5051f1f614b0606a5bdfa Mon Sep 17 00:00:00 2001 From: Bart Schilperoort Date: Wed, 27 Nov 2024 12:03:31 +0100 Subject: [PATCH 7/7] Add basic openapi container test --- tests/src/base/test_model.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/src/base/test_model.py b/tests/src/base/test_model.py index 978fb01f..c4cb00e5 100644 --- a/tests/src/base/test_model.py +++ b/tests/src/base/test_model.py @@ -318,6 +318,23 @@ def test_setup(self, mocked_start_container, tmp_path: Path): timeout=300, ) + @patch("ewatercycle.base.model.start_container") + def test_remotebmi_setup(self, mocked_start_container, tmp_path: Path): + model = ContainerizedModel( + bmi_image="ewatercycle/ewatercycle_dummy:latest", + protocol="openapi", + ) + + model.setup(cfg_dir=str(tmp_path)) + + mocked_start_container.assert_called_once_with( + image="ewatercycle/ewatercycle_dummy:latest", + protocol="openapi", + work_dir=tmp_path, + input_dirs=[], + timeout=300, + ) + @patch("ewatercycle.base.model.start_container") def test_setup_with_additional_input_dirs( self, mocked_start_container, tmp_path: Path