From 30ef4f50a0676d5844130491aff9e375ed194a67 Mon Sep 17 00:00:00 2001 From: oshaughnessy-junior <274802661+oshaughnessy-junior@users.noreply.github.com> Date: Sat, 26 Sep 2026 13:59:47 -0400 Subject: [PATCH 1/2] Fix O4c container image transfers and CPU fallbacks --- CHANGES.rst | 5 + .../Code/RIFT/misc/container_manifest.py | 63 ++++-- .../Code/RIFT/misc/dag_utils_generic.py | 74 ++++--- .../Code/test/test_container_manifest.py | 189 ++++++++++++++++-- docs/container-universe-o4c.md | 32 +++ 5 files changed, 313 insertions(+), 50 deletions(-) create mode 100644 docs/container-universe-o4c.md diff --git a/CHANGES.rst b/CHANGES.rst index d3a26a430..aa5e23a5c 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -2,6 +2,11 @@ --------- Development tree is rift_O4c. + - Container-universe GPU ILE and calibration pilot jobs select image basenames + and transfer the matching full URL, avoiding held jobs from truncated + HTCondor image selectors. CPU jobs, including calibration reweighting, + use a single fallback image without GPU-dependent transfer expressions. + - (rc0) PR 187 stabilizes vectorized time marginalization for loud signals; PR 183 (including PR 163) corrects ChooseFDModes conditioning and J-to-L frame rotation; PR 176 adds ASIMOV 0.7 and PESummary handoff compatibility. diff --git a/MonteCarloMarginalizeCode/Code/RIFT/misc/container_manifest.py b/MonteCarloMarginalizeCode/Code/RIFT/misc/container_manifest.py index 64a6850af..dd99f7bf4 100644 --- a/MonteCarloMarginalizeCode/Code/RIFT/misc/container_manifest.py +++ b/MonteCarloMarginalizeCode/Code/RIFT/misc/container_manifest.py @@ -104,6 +104,14 @@ def _image_runtime_path(image): return image +def _image_basename(image): + """The bare file name an image has once transferred into the job scratch dir. + + Used by the container-universe selector, which MUST NOT contain a ``/``. + """ + return image.rstrip("/").split("/")[-1] + + def _fmt_cap(value): """Format a capability number for a ClassAd expression (e.g. 7.0 -> '7.0').""" return repr(float(value)) @@ -310,17 +318,36 @@ def build_container_image_select(manifest, request_gpu=True): GPU jobs (``request_gpu=True``, the default) get a per-machine selection: an unquoted ``$$([ ... ])`` token. ``$$()`` is HTCondor's *match-time machine-ad substitution* -- the schedd evaluates the bracketed expression against the - matched machine ad and substitutes a literal image string into - ``container_image`` before the job reaches the execution point. Unlike - :func:`build_singularity_image_expr` (an execute-side ClassAd expression that - OSPool glidein pilots read as a literal string and hold on), the pilot only - ever sees a literal URL. ``$$`` in ``container_image`` is HTCondor's - documented mechanism for per-GPU-capability image selection, and it works on - both the CIT-local pool and OSPool glideins. The branch value is the manifest - image *verbatim* (an ``osdf://`` URL the container-universe file-transfer - plugin fetches, or a CVMFS/local path used in place) -- NOT a ``./basename`` - rewrite. ``container_image`` is a single submit command (not a comma list), - so the comma-bearing ``ifThenElse`` form is fine. + matched machine ad and substitutes a literal string before the job reaches the + execution point. Unlike :func:`build_singularity_image_expr` (an execute-side + ClassAd expression that OSPool glidein pilots read as a literal string and hold + on), the pilot only ever sees a literal image name. ``container_image`` is a + single submit command (not a comma list), so the comma-bearing ``ifThenElse`` + form is fine here. + + **The branch values are BASENAMES, not full URLs.** ``condor_submit`` parses + ``container_image`` *before* any ``$$`` expansion and derives the job ad's + ``ContainerImage`` -- the name the image will have in the job scratch dir -- as + the text after the **last** ``/``. A selector containing full paths therefore + gets cut in half, and what survives is not even a valid image name. This is not + theoretical: submitting the full-URL form to the IGWN pool holds the job at the + execute point with:: + + PREPARE_JOB (prepare-hook) failed (reported status 001): + Unable to download or build singularity image cutest_busybox_...sif") ]) + + With no ``/`` in the value, that derivation is a no-op, the whole ``$$`` token + survives into ``ContainerImage``, and the schedd expands it at match time + (``MATCH_EXP_ContainerImage = "rift_container_modern.sif"``) -- verified end to + end on an OSPool glidein. + + Because the selector now names only basenames, the caller MUST also deliver the + matched image itself: add :func:`build_transfer_input_expr` (the comma-free + ternary over the full URLs) to ``transfer_input_files`` **and** emit it as + ``MY.TransferInput`` so it overrides the entry ``condor_submit`` would otherwise + derive from ``container_image``. All images in the family must therefore be + transferable URLs; a family that references an image in place (CVMFS/local path) + cannot be selected this way and raises :class:`ContainerManifestError`. **Non-GPU jobs (``request_gpu=False``) collapse to a SINGLE fixed container**: the plain ``fallback`` image (a literal ``container_image``, no ``$$()``). @@ -338,9 +365,19 @@ def build_container_image_select(manifest, request_gpu=True): by_label = {c["label"]: c for c in manifest["containers"]} fb_image = by_label[manifest["fallback"]]["image"] if not request_gpu: - # Single fixed container: no capability, no $$() -- a plain literal. + # Single fixed container: no capability, no $$() -- a plain literal. This is + # the ordinary single-image path condor_submit handles correctly (it derives + # ContainerImage as the basename, which is exactly right). return fb_image - selector = _build_selector(manifest, lambda c: '"{}"'.format(c["image"])) + in_place = [c["label"] for c in manifest["containers"] if not _image_needs_transfer(c["image"])] + if in_place: + raise ContainerManifestError( + "container universe per-machine selection requires every image in the family " + "to be a transferable URL (e.g. osdf://), because the selector may not contain " + "a '/' -- condor_submit would truncate it. In-place image(s): {}. Either " + "stage those images at a URL, or select a single image instead of a family.".format(", ".join(sorted(in_place))) + ) + selector = _build_selector(manifest, lambda c: '"{}"'.format(_image_basename(c["image"]))) return "$$([ {} ])".format(selector) diff --git a/MonteCarloMarginalizeCode/Code/RIFT/misc/dag_utils_generic.py b/MonteCarloMarginalizeCode/Code/RIFT/misc/dag_utils_generic.py index 3156869a2..979be2220 100644 --- a/MonteCarloMarginalizeCode/Code/RIFT/misc/dag_utils_generic.py +++ b/MonteCarloMarginalizeCode/Code/RIFT/misc/dag_utils_generic.py @@ -2498,10 +2498,9 @@ def write_ILE_sub_simple(tag='integrate', exe=None, log_dir=None, use_eos=False, # Selective transfer: only the matched osdf image is fetched (via the # $$() token, which is comma-free so it survives transfer_input_files # comma-splitting). CVMFS/local images are referenced in place and - # never transferred, so the whole family is never pulled. In container- - # universe mode the image is delivered via container_image itself, so we - # do NOT add the transfer token. - if singularity_transfer_expr and not singularity_container_universe: + # never transferred, so the whole family is never pulled. Container + # universe selects basenames, so it also needs the full-URL transfer. + if singularity_transfer_expr and (request_gpu or not singularity_container_universe): extra_files += [singularity_transfer_expr] elif singularity_image: if 'osdf:' in singularity_image: @@ -2837,6 +2836,15 @@ def write_ILE_sub_simple(tag='integrate', exe=None, log_dir=None, use_eos=False, 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 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 + # bare file name from the access point and fail. Set TransferInput + # directly -- emitted after transfer_input_files, it wins -- so the + # list is exactly ours, with the matched image supplied by the + # comma-free $$() ternary already in extra_files. + ile_job.add_condor_cmd('MY.TransferInput', '"' + fname_str.replace('"', '\\"') + '"') if not transfer_output_files is None: if not isinstance(transfer_output_files, list): @@ -3060,14 +3068,15 @@ 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) - else: - # Selective ($$()) transfer of only the matched osdf image (comma-free so - # it survives transfer_input_files comma-splitting). In container-universe - # mode the image is delivered via container_image itself, so skip this. - _transfer_expr = build_transfer_input_expr(_manifest) - if on_osg and _transfer_expr: - transfer_files += [_transfer_expr] + 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 and (request_gpu or not singularity_container_universe): + transfer_files += [_transfer_expr] if use_singularity: base = os.environ.get('SINGULARITY_BASE_EXE_DIR', '/usr/bin/') @@ -3138,7 +3147,7 @@ def write_calpilot_sub(tag='calpilot', exe=None, log_dir=None, universe="vanilla if use_singularity and singularity_image: job.add_condor_cmd('transfer_executable', 'False') if singularity_container_universe: - # Container universe: the per-machine image is delivered via container_image, + # Container universe: select the basename; transfer the full URL separately, # a $$()-substituted (match-time) literal -- emit it raw/unquoted (a $$() # value must not be wrapped in quotes), with NO MY.SingularityImage / # MY.SingularityBindCVMFS. GPU access is automatic under request_gpus. @@ -3174,8 +3183,14 @@ def write_calpilot_sub(tag='calpilot', exe=None, log_dir=None, universe="vanilla # absolute paths -> condor transfers each to the worker scratch dir by basename, # which is what the stage args (basenames) reference. transfer_files += [wd + "/consolidated_$(macroiteration).composite", ile_args_file] - job.add_condor_cmd('transfer_input_files', ','.join(transfer_files)) + _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 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. + job.add_condor_cmd('MY.TransferInput', '"' + _tif_str.replace('"', '\\"') + '"') job.add_condor_cmd('when_to_transfer_output', 'ON_EXIT') job.add_condor_cmd('transfer_output_files', 'cal_consolidated_$(macroiteration).npz') # Container-family GPU jobs (CALPILOT runs ILE on a GPU): exclude slots that @@ -4323,13 +4338,23 @@ def write_calibration_uncertainty_reweighting_sub(tag='Calib_reweight', exe=None singularity_image_used = "{}".format(singularity_image) # make copy extra_files = [] - if singularity_image: - if 'osdf:' in singularity_image: - singularity_image_used = "./{}".format(singularity_image.split('/')[-1]) - extra_files += [singularity_image] - + singularity_is_family = bool(singularity_image and is_container_manifest(singularity_image)) + singularity_container_universe = bool( + singularity_is_family and use_singularity and os.environ.get('RIFT_CONTAINER_UNIVERSE') + ) + singularity_container_image = None + if singularity_is_family: + _manifest = load_container_manifest(singularity_image) + if singularity_container_universe: + singularity_container_image = build_container_image_select(_manifest, request_gpu=False) + else: + singularity_image_used, fallback_transfer = build_fallback_single_image(_manifest) + if fallback_transfer: + extra_files.append(fallback_transfer) + elif singularity_image and 'osdf:' in singularity_image: + singularity_image_used = "./{}".format(singularity_image.split('/')[-1]) + extra_files.append(singularity_image) - exe = exe or which("calibration_reweighting.py") if exe is None: print(" Calibration Reweighting code not available. ") @@ -4345,7 +4370,7 @@ def write_calibration_uncertainty_reweighting_sub(tag='Calib_reweight', exe=None singularity_base_exe_path = "/usr/bin/" # should not hardcode this ...! exe=singularity_base_exe_path + exe_base - ile_job = CondorDAGJob(universe="vanilla", executable=exe) + ile_job = CondorDAGJob(universe=("container" if singularity_container_universe else "vanilla"), executable=exe) # This is a hack since CondorDAGJob hides the queue property ile_job._CondorJob__queue = ncopies @@ -4361,8 +4386,11 @@ def write_calibration_uncertainty_reweighting_sub(tag='Calib_reweight', exe=None # Compare to https://github.com/lscsoft/lalsuite/blob/master/lalinference/python/lalinference/lalinference_pipe_utils.py ile_job.add_condor_cmd('request_CPUs', str(1)) ile_job.add_condor_cmd('transfer_executable', 'False') - ile_job.add_condor_cmd("MY.SingularityBindCVMFS", 'True') - ile_job.add_condor_cmd("MY.SingularityImage", '"' + singularity_image_used + '"') + if singularity_container_universe: + ile_job.add_condor_cmd("container_image", singularity_container_image) + else: + ile_job.add_condor_cmd("MY.SingularityBindCVMFS", 'True') + ile_job.add_condor_cmd("MY.SingularityImage", '"' + singularity_image_used + '"') ile_job.add_condor_cmd("transfer_output_files", "weight_files") requirements.append("HAS_SINGULARITY=?=TRUE") print(" WARNING: cal reweighting requires bilby. Directories are moved to cal_evelopes") diff --git a/MonteCarloMarginalizeCode/Code/test/test_container_manifest.py b/MonteCarloMarginalizeCode/Code/test/test_container_manifest.py index df9c06a8f..0f5ad53bb 100644 --- a/MonteCarloMarginalizeCode/Code/test/test_container_manifest.py +++ b/MonteCarloMarginalizeCode/Code/test/test_container_manifest.py @@ -2,9 +2,9 @@ Tests for container family manifest parsing and the expression-valued 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). +These run without submitting to a pool. Parser and object-level tests need no +HTCondor installation; effective-job-ad tests invoke condor_submit -dry-run when +available, checking GPU image transfer and CPU fallback behavior. Run directly: python test/test_container_manifest.py Or via pytest: pytest test/test_container_manifest.py @@ -55,6 +55,22 @@ ) +ALL_OSDF_MANIFEST = textwrap.dedent( + """ + version: 1 + fallback: ancient + containers: + - label: ancient + image: osdf:///igwn/sw/rift_ancient_cuda11.sif + cuda_capability_min: 3.0 + cuda_capability_max: 7.0 + - label: modern + image: osdf:///igwn/sw/rift_modern_cuda12.sif + cuda_capability_min: 7.0 + """ +) + + def _write(tmp_path, text, name="fam.yaml"): p = tmp_path / name p.write_text(text) @@ -144,7 +160,8 @@ def test_selectors_are_not_undefined_guarded(tmp_path): m = cm.load_container_manifest(_write(tmp_path, MIXED_MANIFEST)) assert "=?= undefined" not in cm.build_singularity_image_expr(m) assert "=?= undefined" not in cm.build_transfer_input_expr(m) - assert "=?= undefined" not in cm.build_container_image_select(m) + assert "=?= undefined" not in cm.build_container_image_select( + cm.load_container_manifest(_write(tmp_path, ALL_OSDF_MANIFEST, "osdf.yaml"))) def test_capability_defined_requirement(tmp_path): @@ -254,22 +271,38 @@ def test_backward_compat_single_sif(tmp_path, monkeypatch): # --------------------------------------------------------------------------- def test_container_image_select_expression(tmp_path): - m = cm.load_container_manifest(_write(tmp_path, MIXED_MANIFEST)) + m = cm.load_container_manifest(_write(tmp_path, ALL_OSDF_MANIFEST)) expr = cm.build_container_image_select(m) - # a $$() match-time substitution token with VERBATIM image values (osdf URL - # fetched by container universe; cvmfs path used in place) -- NOT a ./basename - # rewrite, and NOT undefined-guarded (Requirements exclusion is used instead) + # A $$() match-time substitution token over BASENAMES. condor_submit derives the + # job ad's ContainerImage as the text after the LAST '/', *before* any $$ + # expansion, so a selector containing a path is truncated and the job holds at the + # execute point ("Unable to download or build singularity image ...sif\") ])", + # observed live on an OSPool glidein). With no '/' the token survives intact and + # the schedd expands it at match time. assert expr.startswith("$$([ ") and expr.endswith(" ])") + assert "/" not in expr # THE invariant assert "=?= undefined" not in expr # not a guess-guard assert "ifThenElse(TARGET.GPUs_Capability >= 7.0," in expr - assert '"osdf:///igwn/rift_modern_cuda12.sif"' in expr # raw osdf URL - assert '"/cvmfs/sw/rift_ancient_cuda11.sif"' in expr # fallback verbatim - assert "./rift_modern_cuda12.sif" not in expr # no basename rewrite + assert '"rift_modern_cuda12.sif"' in expr # basename branch + assert '"rift_ancient_cuda11.sif"' in expr # fallback basename + + +def test_container_image_select_rejects_in_place_images(tmp_path): + # An in-place (CVMFS/local) image can only be named by its full path, which would + # reintroduce the '/' truncation. Refuse loudly rather than emit a submit file + # that holds every job. + m = cm.load_container_manifest(_write(tmp_path, MIXED_MANIFEST)) + with pytest.raises(cm.ContainerManifestError) as exc: + cm.build_container_image_select(m) + assert "ancient" in str(exc.value) + # ... but the CPU-only single-image path is unaffected: it is a plain literal that + # condor_submit handles correctly. + assert cm.build_container_image_select(m, request_gpu=False) == "/cvmfs/sw/rift_ancient_cuda11.sif" def test_integration_container_universe(tmp_path, monkeypatch): # Opt-in container-universe mode: per-machine image via $$()-substituted - # container_image; no MY.SingularityImage / BindCVMFS / $$() transfer token; + # container_image plus a full-URL transfer token; no MY.SingularityImage / BindCVMFS; # universe=container; require_gpus floor still applied. monkeypatch.setenv("RIFT_CONTAINER_UNIVERSE", "1") monkeypatch.delenv("RIFT_REQUIRE_GPUS", raising=False) @@ -282,7 +315,7 @@ def test_integration_container_universe(tmp_path, monkeypatch): arg_str="--foo bar", transfer_files=["../all.net"], use_singularity=True, - singularity_image=_write(tmp_path, MIXED_MANIFEST), + singularity_image=_write(tmp_path, ALL_OSDF_MANIFEST), request_gpu=True, cache_file="local.cache", ) @@ -291,9 +324,19 @@ def test_integration_container_universe(tmp_path, monkeypatch): ci = cmds["container_image"] assert ci.startswith("$$([") # match-time substitution, unquoted assert not ci.startswith('"') + assert "/" not in ci # else condor_submit truncates it assert "MY.SingularityImage" not in cmds # the OSG-breaking attr is gone assert "MY.SingularityBindCVMFS" not in cmds - assert "$$([" not in cmds.get("transfer_input_files", "") # image via container_image, not transfer + + # container_image names only a basename, so the image must arrive by transfer: + # exactly one comma-free $$() token carrying the full URLs. + tif = cmds["transfer_input_files"] + assert tif.count("$$([") == 1 + assert "osdf:///igwn/sw/rift_modern_cuda12.sif" in tif + # ... and TransferInput is pinned, so condor_submit does not append the basename + # selector to it as a bogus extra input file. + assert cmds["MY.TransferInput"] == '"' + tif.replace('"', '\\"') + '"' + assert "Capability >= 3.0" in cmds["require_gpus"] # floor still steers GPUs # GPU family job: still excludes slots that don't advertise the capability attr assert "TARGET.GPUs_Capability =!= undefined" in cmds["requirements"] @@ -365,5 +408,123 @@ def test_integration_cip_legacy_single_image(tmp_path, monkeypatch): assert "require_gpus" not in cmds + + +@pytest.mark.parametrize("role", ["ILE", "CALPILOT"]) +@pytest.mark.parametrize("request_gpu", [True, False]) +def test_container_universe_effective_job_ad(tmp_path, monkeypatch, role, request_gpu): + """Use condor_submit itself: object-level checks miss selector truncation.""" + import json + import shutil + import subprocess + + submit = shutil.which("condor_submit") + if not submit: + pytest.skip("HTCondor required for effective job-ad validation") + 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 == "ILE": + 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 "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() + 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") + else: + monkeypatch.delenv("RIFT_CONTAINER_UNIVERSE", raising=False) + monkeypatch.chdir(tmp_path) + dag = pytest.importorskip("RIFT.misc.dag_utils_generic") + job, _ = dag.write_calibration_uncertainty_reweighting_sub( + tag="Calib_reweight", + log_dir=str(tmp_path) + "/", + exe="/usr/bin/true", + pickle_file=str(tmp_path / "event.pickle"), + posterior_file=str(tmp_path / "posterior.dat"), + transfer_files=[], + use_osg=True, + use_singularity=True, + singularity_image=manifest, + ) + return job, dict(job.condor_cmds) + + +def test_calibration_family_legacy_uses_fallback_not_manifest(tmp_path, monkeypatch): + manifest = _write(tmp_path, ALL_OSDF_MANIFEST) + job, cmds = _make_calibration_job(tmp_path, monkeypatch, manifest, False) + assert job.universe == "vanilla" + assert cmds["MY.SingularityImage"] == '"./rift_ancient_cuda11.sif"' + assert manifest not in cmds["MY.SingularityImage"] + assert "ifThenElse" not in cmds["MY.SingularityImage"] + assert cmds["transfer_input_files"].count( + "osdf:///igwn/sw/rift_ancient_cuda11.sif" + ) == 1 + assert "osdf:///igwn/sw/rift_modern_cuda12.sif" not in cmds["transfer_input_files"] + + +def test_calibration_family_container_universe_uses_fallback_not_manifest( + tmp_path, monkeypatch +): + manifest = _write(tmp_path, ALL_OSDF_MANIFEST) + job, cmds = _make_calibration_job(tmp_path, monkeypatch, manifest, True) + assert job.universe == "container" + assert cmds["container_image"] == "osdf:///igwn/sw/rift_ancient_cuda11.sif" + assert manifest not in cmds["container_image"] + assert "MY.SingularityImage" not in cmds + assert "MY.SingularityBindCVMFS" not in cmds + assert "$$(" not in cmds["container_image"] + assert "rift_modern_cuda12.sif" not in cmds["transfer_input_files"] + + # Exercise the same submit-file emission path as a build-only pipeline run. + job.write_sub_file() + submit = (tmp_path / "Calib_reweight.sub").read_text() + assert "universe = container" in submit + assert "container_image = osdf:///igwn/sw/rift_ancient_cuda11.sif" in submit + assert "fam.yaml" not in submit + + + if __name__ == "__main__": sys.exit(pytest.main([os.path.abspath(__file__), "-v"])) diff --git a/docs/container-universe-o4c.md b/docs/container-universe-o4c.md new file mode 100644 index 000000000..816146876 --- /dev/null +++ b/docs/container-universe-o4c.md @@ -0,0 +1,32 @@ +# Container families on O4c + +With `RIFT_CONTAINER_UNIVERSE=1`, a GPU ILE or calibration-pilot job selects +an image **basename** in `container_image`. The matching full URL is delivered +through `transfer_input_files`; `MY.TransferInput` pins that list so HTCondor +does not add a second, basename-only input. Full URLs inside the image selector +are unsafe: `condor_submit` takes the suffix after the last slash before +match-time expansion, corrupting the expression and holding worker jobs. + +Every image in a GPU container-universe family must be a transferable URL +(such as `osdf://`). Local/CVMFS family entries fail at DAG generation with an +actionable error. Stage them at URLs or select a single image. O4c does not +implement the O4d runtime-selection wrapper. + +CPU jobs, including calibration reweighting, use the declared fallback as a single literal image. They do not +require GPU-capability expansion. CPU ILE retains `--gpu --force-xpy --vectorized` +for the NoLoop likelihood path; scheduler GPU requests are independent of those flags. Plain single-image configurations keep their +existing behavior. The existing GPU capability-floor and defined-capability +requirements remain in force. This change does not alter capability-band +selection or add enforcement of `cuda_capability_max`. + +Run the container tests on a host with HTCondor installed to include actual +submit-file parsing and effective job-ad assertions: + +```sh +PYTHONPATH=MonteCarloMarginalizeCode/Code python -m pytest -q \ + MonteCarloMarginalizeCode/Code/test/test_container_manifest.py +``` + +The HTCondor checks are dry runs: they do not demonstrate image download or +execution on OSPool. The original basename-transfer mechanism was validated +with live OSPool jobs in upstream PR 165. From aeb426d7d1df93fa46ec2d07a440c4ede87c4eb4 Mon Sep 17 00:00:00 2001 From: oshaughnessy-junior <274802661+oshaughnessy-junior@users.noreply.github.com> Date: Sat, 26 Sep 2026 14:18:15 -0400 Subject: [PATCH 2/2] Preserve transfer delimiters for string ILE inputs --- .../Code/RIFT/misc/dag_utils_generic.py | 2 +- .../Code/test/test_container_manifest.py | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/MonteCarloMarginalizeCode/Code/RIFT/misc/dag_utils_generic.py b/MonteCarloMarginalizeCode/Code/RIFT/misc/dag_utils_generic.py index 979be2220..85f0af59c 100644 --- a/MonteCarloMarginalizeCode/Code/RIFT/misc/dag_utils_generic.py +++ b/MonteCarloMarginalizeCode/Code/RIFT/misc/dag_utils_generic.py @@ -2830,7 +2830,7 @@ 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() diff --git a/MonteCarloMarginalizeCode/Code/test/test_container_manifest.py b/MonteCarloMarginalizeCode/Code/test/test_container_manifest.py index 0f5ad53bb..4e16172af 100644 --- a/MonteCarloMarginalizeCode/Code/test/test_container_manifest.py +++ b/MonteCarloMarginalizeCode/Code/test/test_container_manifest.py @@ -410,9 +410,10 @@ def test_integration_cip_legacy_single_image(tmp_path, monkeypatch): -@pytest.mark.parametrize("role", ["ILE", "CALPILOT"]) +@pytest.mark.parametrize("role,transfer_form", [("ILE", "list"), ("ILE", "string"), + ("ILE", "empty-string"), ("CALPILOT", "list")]) @pytest.mark.parametrize("request_gpu", [True, False]) -def test_container_universe_effective_job_ad(tmp_path, monkeypatch, role, request_gpu): +def test_container_universe_effective_job_ad(tmp_path, monkeypatch, role, transfer_form, request_gpu): """Use condor_submit itself: object-level checks miss selector truncation.""" import json import shutil @@ -432,6 +433,10 @@ def test_container_universe_effective_job_ad(tmp_path, monkeypatch, role, reques 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 transfer_form == "string": + common["transfer_files"] = str(tmp_path / "all.net") + elif transfer_form == "empty-string": + common["transfer_files"] = "" if role == "ILE": job, sub = dag.write_ILE_sub_simple(cache_file="local.cache", arg_str="--gpu --force-xpy --vectorized", **common) else: @@ -442,6 +447,9 @@ def test_container_universe_effective_job_ad(tmp_path, monkeypatch, role, reques assert "/" not in cmds["container_image"] assert "osdf:///" in cmds["transfer_input_files"] assert "MY.TransferInput" in cmds + # The URL selector must be its own transfer item, not a suffix of all.net. + expected_prefix = "" if transfer_form == "empty-string" else str(tmp_path / "all.net") + "," + assert cmds["transfer_input_files"].startswith(expected_prefix + "$$([") else: assert cmds["container_image"] == "osdf:///igwn/sw/rift_ancient_cuda11.sif" assert "$$(" not in cmds["transfer_input_files"]