Fix O4d CPU container fallback transfers and ILE transfer separators

MonteCarloMarginalizeCode/Code/RIFT/misc/dag_utils_generic.py

@@ -2530,8 +2530,10 @@ def write_ILE_sub_simple(tag='integrate', exe=None, log_dir=None, use_eos=False,
         # names BASENAMES (it may not contain a '/', or condor_submit truncates
         # it -- see build_container_image_select), so the image itself must be
         # delivered by file transfer.  Runtime-select mode self-fetches inside
-        # the wrapper, so it is the only mode that skips the token.
-        if singularity_transfer_expr and not singularity_runtime_select:
+        # the wrapper. CPU container-universe jobs use a literal fallback image,
+        # which condor_submit transfers without a GPU-capability expression.
+        if (singularity_transfer_expr and not singularity_runtime_select
+                and (request_gpu or not singularity_container_universe)):
             extra_files += [singularity_transfer_expr]
     elif singularity_image:
         if 'osdf:' in singularity_image:
@@ -2871,13 +2873,13 @@ def write_ILE_sub_simple(tag='integrate', exe=None, log_dir=None, use_eos=False,
 
     if not transfer_files is None:
         if not isinstance(transfer_files, list):
-            fname_str=transfer_files + ' '.join(extra_files)
+            fname_str = ','.join(part for part in [transfer_files] + extra_files if part)
         else:
             fname_str = ','.join(transfer_files+extra_files)
         fname_str=fname_str.strip()
         ile_job.add_condor_cmd('transfer_input_files', fname_str)
         ile_job.add_condor_cmd('should_transfer_files','YES')
-        if singularity_container_universe:
+        if singularity_container_universe and request_gpu:
             # condor_submit APPENDS the container_image value to the derived
             # TransferInput.  Our selector names basenames (it may not contain a
             # '/'), so that appended entry would ask the execute point to fetch a
@@ -3118,14 +3120,14 @@ def write_calpilot_sub(tag='calpilot', exe=None, log_dir=None, universe="vanilla
         singularity_require_gpus_floor = build_require_gpus_floor(_manifest)
         singularity_container_universe = bool(use_singularity and os.environ.get('RIFT_CONTAINER_UNIVERSE'))
         if singularity_container_universe:
-            singularity_container_image_select = build_container_image_select(_manifest)
+            singularity_container_image_select = build_container_image_select(_manifest, request_gpu=request_gpu)
         # Selective ($$()) transfer of only the matched osdf image (comma-free so it
         # survives transfer_input_files comma-splitting).  Container universe needs it
         # too: its container_image selector names BASENAMES (it may not contain a '/',
         # or condor_submit truncates it), so the image arrives by file transfer.
         # (container universe requires use_singularity, which already implies on_osg)
         _transfer_expr = build_transfer_input_expr(_manifest)
-        if on_osg and _transfer_expr:
+        if on_osg and _transfer_expr and (request_gpu or not singularity_container_universe):
             transfer_files += [_transfer_expr]
 
     if use_singularity:
@@ -3236,7 +3238,7 @@ def write_calpilot_sub(tag='calpilot', exe=None, log_dir=None, universe="vanilla
         _tif_str = ','.join(transfer_files)
         job.add_condor_cmd('transfer_input_files', _tif_str)
         job.add_condor_cmd('should_transfer_files', 'YES')
-        if singularity_container_universe:
+        if singularity_container_universe and request_gpu:
             # condor_submit APPENDS the container_image value to the derived
             # TransferInput; our selector names basenames, so that entry would ask
             # the execute point to fetch a bare file name and fail.  Pin the list.

MonteCarloMarginalizeCode/Code/test/test_container_manifest.py

@@ -3,8 +3,11 @@
 SingularityImage / selective-transfer / require_gpus wiring.
 
 These run without a real HTCondor pool: the parser + expression builders are
-pure, and the integration test inspects the generated ``condor_cmds`` on the
-job object returned by ``write_ILE_sub_simple`` (no .sub file or condor needed).
+pure, and the integration tests inspect the generated ``condor_cmds`` on the
+job object returned by ``write_ILE_sub_simple``, plus the submit file the
+backend renders in pure python.  Nothing here skips when HTCondor is absent;
+``test_container_universe_effective_job_ad`` additionally hands that submit file
+to ``condor_submit -dry-run`` wherever that executable exists.
 
 Run directly:  python test/test_container_manifest.py
 Or via pytest: pytest test/test_container_manifest.py
@@ -411,6 +414,109 @@ def test_integration_cip_legacy_single_image(tmp_path, monkeypatch):
     assert "require_gpus" not in cmds
 
 
+@pytest.mark.parametrize("role", ["ILE", "ILE_STRING", "CALPILOT"])
+@pytest.mark.parametrize("request_gpu", [True, False])
+def test_container_universe_effective_job_ad(tmp_path, monkeypatch, role, request_gpu):
+    """Check the SUBMITTED text, not just the job object: object-level checks
+    miss selector truncation.
+
+    Two legs, neither of which skips.  The submit-file leg runs EVERYWHERE: the
+    HTCondor backend has a pure-python submit-file renderer, so the selector can
+    always be read back as written, and a value that acquires a newline (which
+    silently truncates the selector into a different, still-plausible submit
+    description) fails here with no HTCondor present.  The condor_submit leg --
+    the only one that also proves condor's own parser resolves the expression --
+    additionally runs wherever condor_submit is on PATH, i.e. a submit host or a
+    Condor-enabled gate.
+
+    Deliberately NOT a pytest.skip when condor_submit is absent.  This file is a
+    member of .travis/test-core-units.sh, whose runner has no HTCondor, so six
+    parametrized skips would spend that gate's skip budget on cases that can
+    never run there -- and the gate's outcome accounting, which exists to stop a
+    skip absorbing a lost check, would fail on a check that is not lost.
+    """
+    import json
+
+    from RIFT.misc import dag_utils_generic as dag
+
+    monkeypatch.setenv("RIFT_CONTAINER_UNIVERSE", "1")
+    monkeypatch.chdir(tmp_path)
+    manifest = _write(tmp_path, ALL_OSDF_MANIFEST)
+    (tmp_path / "all.net").write_text("fixture\n")
+    (tmp_path / "args_ile.txt").write_text("--n-max 1\n")
+    (tmp_path / "consolidated_0.composite").write_text("fixture\n")
+    common = dict(tag=role, exe="/usr/bin/true", log_dir=str(tmp_path) + "/",
+                  use_singularity=True, singularity_image=manifest,
+                  request_gpu=request_gpu, transfer_files=[str(tmp_path / "all.net")])
+    if role.startswith("ILE"):
+        if role == "ILE_STRING":
+            common["transfer_files"] = str(tmp_path / "all.net")
+        job, sub = dag.write_ILE_sub_simple(cache_file="local.cache", arg_str="--gpu --force-xpy --vectorized", **common)
+    else:
+        job, sub = dag.write_calpilot_sub(working_directory=str(tmp_path),
+                                         ile_args_file=str(tmp_path / "args_ile.txt"), **common)
+    cmds = dict(job.condor_cmds)
+    if request_gpu:
+        assert "/" not in cmds["container_image"]
+        assert str(tmp_path / "all.net") + "," in cmds["transfer_input_files"]
+        assert "osdf:///" in cmds["transfer_input_files"]
+        assert "MY.TransferInput" in cmds
+    else:
+        assert cmds["container_image"] == "osdf:///igwn/sw/rift_ancient_cuda11.sif"
+        assert "$$(" not in cmds["transfer_input_files"]
+        assert "MY.TransferInput" not in cmds
+    job.add_condor_cmd("macroevent", "0")
+    job.add_condor_cmd("macroiteration", "0")
+    job.add_condor_cmd("macroiterationprev", "0")
+    job.write_sub_file()
+
+    # Leg 1, everywhere: the values as WRITTEN.  Keys are matched case-folded
+    # because the renderer used depends on whether the htcondor bindings are
+    # importable, and only the VALUES are the contract here.
+    written = {}
+    with open(str(sub)) as fh:
+        sub_text = fh.read()
+    for line in sub_text.splitlines():
+        key, sep, value = line.partition("=")
+        if not sep:
+            continue                       # 'queue N', and any truncated remnant
+        key = key.strip().lower()
+        if key.startswith("+"):
+            key = "my." + key[1:]          # the two spellings of a custom attribute
+        written.setdefault(key, value.strip())
+    assert written["container_image"] == cmds["container_image"]
+    if request_gpu:
+        # A selector truncated on write reaches this comparison short, not wrong.
+        assert written["transfer_input_files"] == cmds["transfer_input_files"]
+        assert written["my.transferinput"] == cmds["MY.TransferInput"]
+    else:
+        assert "$$(" not in written["transfer_input_files"]
+        assert "my.transferinput" not in written
+
+    # Leg 2, Condor-enabled environments only: condor's own parser resolves the
+    # expression and reports what the job would actually run with.
+    submit = shutil.which("condor_submit")
+    if submit:
+        ad_path = tmp_path / "effective.ad"
+        result = subprocess.run([submit, "-disable", "-dry-run", str(ad_path), str(sub)],
+                                capture_output=True, text=True, timeout=30)
+        assert result.returncode == 0, result.stdout + result.stderr
+        ad = {}
+        for line in ad_path.read_text().splitlines():
+            key, sep, value = line.partition("=")
+            key = key.strip()
+            value = value.strip()
+            if sep and key in ("ContainerImage", "TransferInput"):
+                ad[key] = json.loads(value)
+        if request_gpu:
+            assert ad["ContainerImage"] == cmds["container_image"]
+            assert ad["TransferInput"] == cmds["transfer_input_files"].replace("$(macroiteration)", "0")
+            assert "ifThenElse" not in ad["TransferInput"]  # no spurious basename input
+        else:
+            assert ad["ContainerImage"] == "rift_ancient_cuda11.sif"
+            assert "$$(" not in ad["TransferInput"]
+
+
 def _make_calibration_job(tmp_path, monkeypatch, manifest, container_universe):
     if container_universe:
         monkeypatch.setenv("RIFT_CONTAINER_UNIVERSE", "1")