Fix O4c container-universe transfers and CPU image fallbacks

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.

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)
 
 

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:
@@ -2831,12 +2830,21 @@ 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 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")