From 5081d6ea394f533da65bfb41503fb235924559d7 Mon Sep 17 00:00:00 2001 From: gvdoord Date: Fri, 24 Jul 2020 22:21:01 +0200 Subject: [PATCH 1/5] Added extra volumes to singularity client --- grpc4bmi/bmi_client_singularity.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/grpc4bmi/bmi_client_singularity.py b/grpc4bmi/bmi_client_singularity.py index 5a9677e..6a88c76 100644 --- a/grpc4bmi/bmi_client_singularity.py +++ b/grpc4bmi/bmi_client_singularity.py @@ -41,12 +41,22 @@ class BmiClientSingularity(BmiClient): input_dir (str): Directory for input files of model output_dir (str): Directory for input files of model timeout (int): Seconds to wait for gRPC client to connect to server + extra_volumes (Dict[str,str]): Extra volumes to attach to Singularity container. + + The key is either the hosts path or a volume name and the value the mounted volume inside the container. + Contrary to docker client, extra volumes are always read/write + + For example: + + .. code-block:: python + + {'/data/shared/forcings/': /data/forcings'} """ INPUT_MOUNT_POINT = "/data/input" OUTPUT_MOUNT_POINT = "/data/output" - def __init__(self, image, input_dir=None, output_dir=None, timeout=None): + def __init__(self, image, input_dir=None, output_dir=None, timeout=None, extra_volumes=None): check_singularity_version() host = 'localhost' port = BmiClient.get_unique_port(host) @@ -54,9 +64,12 @@ def __init__(self, image, input_dir=None, output_dir=None, timeout=None): "singularity", "run", ] + mount_points = {} if extra_volumes is None else extra_volumes if input_dir is not None: + mount_points[input_dir] = BmiClientSingularity.INPUT_MOUNT_POINT self.input_dir = abspath(input_dir) - args += ["--bind", input_dir + ':' + BmiClientSingularity.INPUT_MOUNT_POINT] + if any(mount_points): + args += ["--bind", ','.join([hp + ':' + ip for hp, ip in mount_points.items()])] if output_dir is not None: self.output_dir = abspath(output_dir) try: From 8763836932f005e1f5ebb8c1ac1fe35c83c522f0 Mon Sep 17 00:00:00 2001 From: Stefan Verhoeven Date: Mon, 27 Jul 2020 09:51:39 +0200 Subject: [PATCH 2/5] Added test for singularity extra volumes --- grpc4bmi/bmi_client_singularity.py | 4 ++-- test/test_singularity.py | 19 ++++++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/grpc4bmi/bmi_client_singularity.py b/grpc4bmi/bmi_client_singularity.py index 6a88c76..f254923 100644 --- a/grpc4bmi/bmi_client_singularity.py +++ b/grpc4bmi/bmi_client_singularity.py @@ -43,8 +43,8 @@ class BmiClientSingularity(BmiClient): timeout (int): Seconds to wait for gRPC client to connect to server extra_volumes (Dict[str,str]): Extra volumes to attach to Singularity container. - The key is either the hosts path or a volume name and the value the mounted volume inside the container. - Contrary to docker client, extra volumes are always read/write + The key is the hosts path and the value the mounted volume inside the container. + Contrary to Docker client, extra volumes are always read/write For example: diff --git a/test/test_singularity.py b/test/test_singularity.py index 39732e2..0e3fce8 100644 --- a/test/test_singularity.py +++ b/test/test_singularity.py @@ -6,7 +6,7 @@ from nbformat.v4 import new_notebook, new_code_cell from grpc4bmi.bmi_client_singularity import BmiClientSingularity -from grpc4bmi.reserve import reserve_grid_padding +from grpc4bmi.reserve import reserve_grid_padding, reserve_values IMAGE_NAME = "docker://ewatercycle/walrus-grpc4bmi:v0.3.1" @@ -18,6 +18,15 @@ def walrus_model(tmp_path, walrus_input): del model +@pytest.fixture() +def walrus_model_with_extra_volume(walrus_input_on_extra_volume): + (input_dir, docker_extra_volumes) = walrus_input_on_extra_volume + extra_volumes = {k: v['bind'] for k, v in docker_extra_volumes.items()} + model = BmiClientSingularity(image=IMAGE_NAME, input_dir=str(input_dir), extra_volumes=extra_volumes) + yield model + del model + + class TestBmiClientDocker: def test_component_name(self, walrus_model): assert walrus_model.get_component_name() == 'WALRUS' @@ -35,6 +44,14 @@ def test_get_grid_origin(self, walrus_input, walrus_model): grid_id = walrus_model.get_var_grid('Q') assert len(walrus_model.get_grid_origin(grid_id, reserve_grid_padding(walrus_model, grid_id))) == 2 + def test_extra_volumes(self, walrus_model_with_extra_volume): + walrus_model_with_extra_volume.initialize('/data/input/config.yml') + walrus_model_with_extra_volume.update() + + # After initialization and update the forcings have been read from the extra volume + result = reserve_values(walrus_model_with_extra_volume, 'Q') + assert len(walrus_model_with_extra_volume.get_value('Q', result)) == 1 + @pytest.fixture def notebook(): From a8525a89c7b705e202f1aa421a2c1f544544dfaf Mon Sep 17 00:00:00 2001 From: Stefan Verhoeven Date: Mon, 27 Jul 2020 10:25:10 +0200 Subject: [PATCH 3/5] Cast PosixPath to str --- test/test_singularity.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_singularity.py b/test/test_singularity.py index 0e3fce8..53a267d 100644 --- a/test/test_singularity.py +++ b/test/test_singularity.py @@ -21,7 +21,7 @@ def walrus_model(tmp_path, walrus_input): @pytest.fixture() def walrus_model_with_extra_volume(walrus_input_on_extra_volume): (input_dir, docker_extra_volumes) = walrus_input_on_extra_volume - extra_volumes = {k: v['bind'] for k, v in docker_extra_volumes.items()} + extra_volumes = {str(k): str(v['bind']) for k, v in docker_extra_volumes.items()} model = BmiClientSingularity(image=IMAGE_NAME, input_dir=str(input_dir), extra_volumes=extra_volumes) yield model del model From 740ab962d1894660461efb41c66b5acbc60c88d5 Mon Sep 17 00:00:00 2001 From: Stefan Verhoeven Date: Mon, 27 Jul 2020 14:20:09 +0200 Subject: [PATCH 4/5] Pin protobuf to prevent 4.0.0rc2 install --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index a68fa51..0d4549b 100644 --- a/setup.py +++ b/setup.py @@ -29,7 +29,7 @@ def read(fname): "grpcio-reflection==1.27.2", "grpcio-status==1.27.2", "googleapis-common-protos>=1.5.5", - "protobuf", + "protobuf==3.12.2", "numpy", "docker", "bmipy", From 5bdbb3e237c38bce6ef2316de8a4b75c879b3c9f Mon Sep 17 00:00:00 2001 From: Stefan Verhoeven Date: Mon, 27 Jul 2020 14:38:44 +0200 Subject: [PATCH 5/5] Container clients need get_value_ptr instead of get_value_ref --- grpc4bmi/bmi_client_docker.py | 3 ++- grpc4bmi/bmi_client_singularity.py | 2 +- test/test_docker.py | 4 ++-- test/test_singularity.py | 4 ++-- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/grpc4bmi/bmi_client_docker.py b/grpc4bmi/bmi_client_docker.py index 44e6e19..cc12604 100644 --- a/grpc4bmi/bmi_client_docker.py +++ b/grpc4bmi/bmi_client_docker.py @@ -11,6 +11,7 @@ class LogsException(Exception): pass + class DeadDockerContainerException(ChildProcessError): """ Exception for when a Docker container has died. @@ -115,7 +116,7 @@ def initialize(self, filename): fn = stage_config_file(filename, self.input_dir, self.input_mount_point) super(BmiClientDocker, self).initialize(fn) - def get_value_ref(self, var_name): + def get_value_ptr(self, var_name): raise NotImplementedError("Cannot exchange memory references across process boundary") def logs(self): diff --git a/grpc4bmi/bmi_client_singularity.py b/grpc4bmi/bmi_client_singularity.py index f254923..8ba6a62 100644 --- a/grpc4bmi/bmi_client_singularity.py +++ b/grpc4bmi/bmi_client_singularity.py @@ -95,5 +95,5 @@ def initialize(self, filename): fn = stage_config_file(filename, self.input_dir, self.INPUT_MOUNT_POINT, home_mounted=True) super(BmiClientSingularity, self).initialize(fn) - def get_value_ref(self, var_name): + def get_value_ptr(self, var_name): raise NotImplementedError("Cannot exchange memory references across process boundary") diff --git a/test/test_docker.py b/test/test_docker.py index 5147463..f95dff8 100644 --- a/test/test_docker.py +++ b/test/test_docker.py @@ -52,9 +52,9 @@ def test_initialize(self, walrus_input, walrus_model): walrus_model.initialize(str(walrus_input)) assert walrus_model.get_current_time() == walrus_model.get_start_time() - def test_get_value_ref(self, walrus_model): + def test_get_value_ptr(self, walrus_model): with pytest.raises(NotImplementedError): - walrus_model.get_value_ref('Q') + walrus_model.get_value_ptr('Q') def test_extra_volume(self, walrus_model_with_extra_volume): walrus_model_with_extra_volume.initialize('/data/input/config.yml') diff --git a/test/test_singularity.py b/test/test_singularity.py index 53a267d..3706be8 100644 --- a/test/test_singularity.py +++ b/test/test_singularity.py @@ -35,9 +35,9 @@ def test_initialize(self, walrus_input, walrus_model): walrus_model.initialize(str(walrus_input)) assert walrus_model.get_current_time() == walrus_model.get_start_time() - def test_get_value_ref(self, walrus_model): + def test_get_value_ptr(self, walrus_model): with pytest.raises(NotImplementedError): - walrus_model.get_value_ref('Q') + walrus_model.get_value_ptr('Q') def test_get_grid_origin(self, walrus_input, walrus_model): walrus_model.initialize(str(walrus_input))