MPI: move hints/reqs + DockerRequirement checking into Process._init_job()

Author: mr-c

cwltool/command_line_tool.py

@@ -431,28 +431,6 @@ def make_job_runner(self, runtimeContext: RuntimeContext) -> type[JobBase]:
                 return SingularityCommandLineJob
             elif runtimeContext.user_space_docker_cmd:
                 return UDockerCommandLineJob
-            if mpiReq is not None:
-                if mpiRequired:
-                    if dockerRequired:
-                        raise UnsupportedRequirement(
-                            "No support for Docker and MPIRequirement both being required"
-                        )
-                    else:
-                        _logger.warning(
-                            "MPI has been required while Docker is hinted, discarding Docker hint(s)"
-                        )
-                        self.hints = [h for h in self.hints if h["class"] != "DockerRequirement"]
-                        return CommandLineJob
-                else:
-                    if dockerRequired:
-                        _logger.warning(
-                            "Docker has been required while MPI is hinted, discarding MPI hint(s)"
-                        )
-                        self.hints = [h for h in self.hints if h["class"] != MPIRequirementName]
-                    else:
-                        raise UnsupportedRequirement(
-                            "Both Docker and MPI have been hinted - don't know what to do"
-                        )
             if runtimeContext.podman:
                 return PodmanCommandLineJob
             return DockerCommandLineJob

cwltool/process.py

@@ -810,12 +810,43 @@ def inc(d: list[int]) -> None:
         tmpdir = ""
         stagedir = ""
 
-        docker_req, _ = self.get_requirement("DockerRequirement")
+        docker_req, docker_required = self.get_requirement("DockerRequirement")
         default_docker = None
+        mpi_req, mpi_required = self.get_requirement(MPIRequirementName)
 
         if docker_req is None and runtime_context.default_container:
             default_docker = runtime_context.default_container
 
+        if (
+            docker_req is not None
+            and runtime_context.use_container
+            and not runtime_context.singularity
+            and not runtime_context.user_space_docker_cmd
+            and mpi_req is not None
+        ):
+            if mpi_required:
+                if docker_required:
+                    raise UnsupportedRequirement(
+                        "No support for Docker and MPIRequirement both being required"
+                    )
+                else:
+                    _logger.warning(
+                        "MPI has been required while Docker is hinted, discarding Docker hint(s)"
+                    )
+                    self.hints = [h for h in self.hints if h["class"] != "DockerRequirement"]
+                    docker_req = None
+                    docker_required = False
+            else:
+                if docker_required:
+                    _logger.warning(
+                        "Docker has been required while MPI is hinted, discarding MPI hint(s)"
+                    )
+                    self.hints = [h for h in self.hints if h["class"] != MPIRequirementName]
+                else:
+                    raise UnsupportedRequirement(
+                        "Both Docker and MPI have been hinted - don't know what to do"
+                    )
+
         if (docker_req or default_docker) and runtime_context.use_container:
             if docker_req is not None:
                 # Check if docker output directory is absolute

tests/test_mpi.py

@@ -7,20 +7,21 @@
 from importlib.resources import files
 from io import StringIO
 from pathlib import Path
-from typing import Any, Optional
+from typing import Any, Optional, cast
 
 import pytest
 from ruamel.yaml.comments import CommentedMap, CommentedSeq
 from schema_salad.avro.schema import Names
+from schema_salad.ref_resolver import file_uri
 from schema_salad.utils import yaml_no_ts
 
 import cwltool.load_tool
 import cwltool.singularity
 import cwltool.udocker
-from cwltool.command_line_tool import CommandLineTool
-from cwltool.context import LoadingContext, RuntimeContext
+from cwltool.context import RuntimeContext
 from cwltool.main import main
 from cwltool.mpi import MpiConfig, MPIRequirementName
+from cwltool.command_line_tool import CommandLineTool
 
 from .util import get_data, working_directory
 
@@ -292,15 +293,22 @@ def schema_ext11() -> Generator[Names, None, None]:
 
 mpiReq = CommentedMap({"class": MPIRequirementName, "processes": 1})
 containerReq = CommentedMap({"class": "DockerRequirement"})
-basetool = CommentedMap({"cwlVersion": "v1.1", "inputs": CommentedSeq(), "outputs": CommentedSeq()})
+basetool = CommentedMap(
+    {
+        "cwlVersion": "v1.1",
+        "class": "CommandLineTool",
+        "inputs": CommentedSeq(),
+        "outputs": CommentedSeq(),
+    }
+)
 
 
 def mk_tool(
     schema: Names,
     opts: list[str],
     reqs: Optional[list[CommentedMap]] = None,
     hints: Optional[list[CommentedMap]] = None,
-) -> tuple[LoadingContext, RuntimeContext, CommentedMap]:
+) -> tuple[RuntimeContext, CommandLineTool]:
     tool = basetool.copy()
 
     if reqs is not None:
@@ -313,50 +321,53 @@ def mk_tool(
     rc = RuntimeContext(vars(args))
     lc = cwltool.main.setup_loadingContext(None, rc, args)
     lc.avsc_names = schema
-    return lc, rc, tool
+    tool["id"] = file_uri(os.path.abspath("./mktool.cwl"))
+    assert lc.loader is not None
+    lc.loader.idx[tool["id"]] = tool
+    return rc, cast(CommandLineTool, cwltool.load_tool.load_tool(tool, lc))
 
 
 def test_singularity(schema_ext11: Names) -> None:
-    lc, rc, tool = mk_tool(schema_ext11, ["--singularity"], reqs=[mpiReq, containerReq])
-    clt = CommandLineTool(tool, lc)
+    rc, clt = mk_tool(schema_ext11, ["--singularity"], reqs=[mpiReq, containerReq])
+    clt._init_job({}, rc)
     jr = clt.make_job_runner(rc)
     assert jr is cwltool.singularity.SingularityCommandLineJob
 
 
 def test_udocker(schema_ext11: Names) -> None:
-    lc, rc, tool = mk_tool(schema_ext11, ["--udocker"], reqs=[mpiReq, containerReq])
-    clt = CommandLineTool(tool, lc)
+    rc, clt = mk_tool(schema_ext11, ["--udocker"], reqs=[mpiReq, containerReq])
+    clt._init_job({}, rc)
     jr = clt.make_job_runner(rc)
     assert jr is cwltool.udocker.UDockerCommandLineJob
 
 
 def test_docker_hint(schema_ext11: Names) -> None:
     # Docker hint, MPI required
-    lc, rc, tool = mk_tool(schema_ext11, [], hints=[containerReq], reqs=[mpiReq])
-    clt = CommandLineTool(tool, lc)
+    rc, clt = mk_tool(schema_ext11, [], hints=[containerReq], reqs=[mpiReq])
+    clt._init_job({}, rc)
     jr = clt.make_job_runner(rc)
     assert jr is cwltool.job.CommandLineJob
 
 
 def test_docker_required(schema_ext11: Names) -> None:
     # Docker required, MPI hinted
-    lc, rc, tool = mk_tool(schema_ext11, [], reqs=[containerReq], hints=[mpiReq])
-    clt = CommandLineTool(tool, lc)
+    rc, clt = mk_tool(schema_ext11, [], reqs=[containerReq], hints=[mpiReq])
+    clt._init_job({}, rc)
     jr = clt.make_job_runner(rc)
     assert jr is cwltool.docker.DockerCommandLineJob
 
 
 def test_docker_mpi_both_required(schema_ext11: Names) -> None:
     # Both required - error
-    lc, rc, tool = mk_tool(schema_ext11, [], reqs=[mpiReq, containerReq])
-    clt = CommandLineTool(tool, lc)
+    rc, clt = mk_tool(schema_ext11, [], reqs=[mpiReq, containerReq])
     with pytest.raises(cwltool.errors.UnsupportedRequirement):
-        clt.make_job_runner(rc)
+        clt._init_job({}, rc)
+    clt.make_job_runner(rc)
 
 
 def test_docker_mpi_both_hinted(schema_ext11: Names) -> None:
     # Both hinted - error
-    lc, rc, tool = mk_tool(schema_ext11, [], hints=[mpiReq, containerReq])
-    clt = CommandLineTool(tool, lc)
+    rc, clt = mk_tool(schema_ext11, [], hints=[mpiReq, containerReq])
     with pytest.raises(cwltool.errors.UnsupportedRequirement):
-        clt.make_job_runner(rc)
+        clt._init_job({}, rc)
+    clt.make_job_runner(rc)