diff --git a/babs/input_dataset.py b/babs/input_dataset.py index 45ec2ed6..45aba31d 100644 --- a/babs/input_dataset.py +++ b/babs/input_dataset.py @@ -24,6 +24,7 @@ def __init__( is_zipped, unzipped_path_containing_subject_dirs=None, required_files=None, + common_paths=None, processing_level=None, babs_project_analysis_path=None, ): @@ -43,6 +44,13 @@ def __init__( when unzipped, this string precedes the subject directories required_files: list of str or None list of required files in the input dataset + common_paths: list of str or None + *explicit* dataset-root-relative paths to include in the sparse-checkout for + every job, in addition to the per-subject (and per-session) path AND the + automatic BIDS-inheritance grab (see ``participant_job.sh.jinja2``: every job + also pulls all metadata blobs sitting at the dataset root, plus the subject + tier for session-level jobs). Use this only for a non-inherited file the grab + doesn't reach (e.g. a shared ``sourcedata/.../nidm.ttl``). Defaults to ``[]``. processing_level: {'subject', 'session'} or None whether processing is done on a subject-wise or session-wise basis babs_project_analysis_path: str or None @@ -57,6 +65,7 @@ def __init__( else: self.is_zipped = bool(is_zipped) self.required_files = required_files + self.common_paths = [] if common_paths is None else common_paths if processing_level not in ['subject', 'session']: raise ValueError('invalid `processing_level`!') self.processing_level = processing_level @@ -269,6 +278,7 @@ def as_dict(self): 'is_zipped': self.is_zipped, 'unzipped_path_containing_subject_dirs': unzipped_path, 'required_files': self.required_files, + 'common_paths': self.common_paths, 'processing_level': self.processing_level, 'babs_project_analysis_path': self.babs_project_analysis_path, } @@ -433,4 +443,5 @@ def __init__(self, input_dataset): input_dataset.unzipped_path_containing_subject_dirs ) self.required_files = input_dataset.required_files + self.common_paths = input_dataset.common_paths self.processing_level = input_dataset.processing_level diff --git a/babs/templates/participant_job.sh.jinja2 b/babs/templates/participant_job.sh.jinja2 index c7ff6dc5..9468a8bf 100644 --- a/babs/templates/participant_job.sh.jinja2 +++ b/babs/templates/participant_job.sh.jinja2 @@ -81,24 +81,53 @@ git checkout -f # pull down only needed session path and explicit dataset-level metadata: echo "# Pull down the input session but don't retrieve data contents:" + +# resolve_tier lists the files of ONE directory tier of an input subdataset. +# git ls-tree reads the committed tree (independent of sparse/checkout state) and is +# non-recursive, so it is anchored to the named tier and never descends into other +# subjects. In valid BIDS no data files sit at the dataset root or directly under +# sub-XX/, so every blob at those tiers is BIDS-inherited metadata (task/modality +# .json sidecars, participants/sessions .tsv, .bval/.bvec, ...). We resolve to literal +# paths because `datalad get`/`git sparse-checkout` cannot take a glob. +resolve_tier() { # $1 = subdataset path, $2 = tier dir relative to its root ('' = root) + git -C "$1" ls-tree HEAD ${2:+"$2/"} 2>/dev/null | awk '$2 == "blob" { sub(/^[^\t]*\t/, ""); print }' +} + +DATALAD_INPUTS=() {% for input_dataset in input_datasets %} {% if not input_dataset['is_zipped'] %} -datalad get -n "{{ input_dataset['path_in_babs'] }}/{% raw %}${subid}{% endraw %}{% if processing_level == 'session' %}/{% raw %}${sesid}{% endraw %}{% endif %}" +datalad get -n "{{ input_dataset['path_in_babs'] }}/${subid}{% if processing_level == 'session' %}/${sesid}{% endif %}" -datalad get -n "{{ input_dataset['path_in_babs'] }}/dataset_description.json" -{% else %} -datalad get -n "{{ input_dataset['path_in_babs'] }}" +# BIDS inheritance: pull metadata from the tiers ABOVE this job's checkout -- the +# dataset root always, plus the subject tier (sub-XX/) for session-level jobs, whose +# sub-XX/ses-YY checkout would otherwise miss files sitting directly under sub-XX/. +inherited=() +while IFS= read -r _f; do inherited+=( "$_f" ); done < <(resolve_tier "{{ input_dataset['path_in_babs'] }}" "") +{% if processing_level == 'session' %} +while IFS= read -r _f; do inherited+=( "$_f" ); done < <(resolve_tier "{{ input_dataset['path_in_babs'] }}" "${subid}") {% endif %} +for rel in ${inherited[@]+"${inherited[@]}"}; do + echo "# Getting inherited metadata: {{ input_dataset['path_in_babs'] }}/${rel}" + datalad get -n "{{ input_dataset['path_in_babs'] }}/${rel}" + DATALAD_INPUTS+=( -i "{{ input_dataset['path_in_babs'] }}/${rel}" ) +done +{% for common_path in input_dataset['common_paths'] %} +echo "# Getting common path: {{ input_dataset['path_in_babs'] }}/{{ common_path }}" +datalad get -n "{{ input_dataset['path_in_babs'] }}/{{ common_path }}" {% endfor %} -# Restrict each BIDS input subdataset to the current subject so BIDS apps that index the full dataset (e.g. pybids BIDSLayout) don't try to read other subjects' files, which may not be retrieved -{% for input_dataset in input_datasets %} -{% if not input_dataset['is_zipped'] %} +# Restrict this subdataset to the subject/session subtree + inherited metadata + any +# explicit common_paths, so BIDS apps that index the dataset (e.g. pybids BIDSLayout) +# don't read other subjects' files, which may not be retrieved. if [ -d "{{ input_dataset['path_in_babs'] }}/.git" ]; then + sparse=( "${subid}{% if processing_level == 'session' %}/${sesid}{% endif %}" {% for common_path in input_dataset['common_paths'] %}'{{ common_path }}' {% endfor %}) + sparse+=( ${inherited[@]+"${inherited[@]}"} ) ( cd "{{ input_dataset['path_in_babs'] }}" && \ - ( git sparse-checkout init --no-cone 2>/dev/null && \ - { echo "{% raw %}${subid}{% endraw %}{% if processing_level == 'session' %}/{% raw %}${sesid}{% endraw %}{% endif %}"; echo 'dataset_description.json'; } | git sparse-checkout set --stdin 2>/dev/null ) ) || true + git sparse-checkout init --no-cone 2>/dev/null && \ + printf '%s\n' "${sparse[@]}" | git sparse-checkout set --stdin 2>/dev/null ) || true fi +{% else %} +datalad get -n "{{ input_dataset['path_in_babs'] }}" {% endif %} {% endfor %} @@ -135,11 +164,13 @@ datalad run \ {% for input_dataset in input_datasets %} {% if not input_dataset['is_zipped'] %} -i "{{ input_dataset['unzipped_path_containing_subject_dirs'] }}/{% raw %}${subid}{% endraw %}{% if processing_level == 'session' %}/{% raw %}${sesid}{% endraw %}{% endif %}" \ - -i "{{ input_dataset['path_in_babs'] }}/dataset_description.json" \ +{% for common_path in input_dataset['common_paths'] %} -i "{{ input_dataset['path_in_babs'] }}/{{ common_path }}" \ +{% endfor %} {% else %} -i "${%raw%}{{%endraw%}{{ input_dataset['name'].upper() }}_ZIP{%raw%}}{%endraw%}" \ {% endif %} {% endfor %} + ${DATALAD_INPUTS[@]+"${DATALAD_INPUTS[@]}"} \ {% for image_path in container_image_paths %} -i "{{ image_path }}" \ {% endfor %} diff --git a/docs/preparation_config_yaml_file.rst b/docs/preparation_config_yaml_file.rst index 767a95ac..da5edd03 100644 --- a/docs/preparation_config_yaml_file.rst +++ b/docs/preparation_config_yaml_file.rst @@ -29,6 +29,7 @@ Sections in the configuration YAML file * **all_results_in_one_zip**: whether to zip all results in one zip file; * **zip_foldernames**: the results foldername(s) to be zipped; * **required_files**: to only keep subjects (sessions) that have this list of required files in input dataset(s); +* **common_paths**: extra, *non-inherited* files to stage for every job (BIDS metadata inheritance is automatic — see :ref:`bids-inheritance`); e.g. a shared ``sourcedata/NIDM/nidm.ttl``; * **alert_log_messages**: alert messages in the log files that may be helpful for debugging errors in failed jobs; Among these sections, these sections are optional: @@ -40,6 +41,7 @@ Among these sections, these sections are optional: * You must include this section if there are more one input dataset. * **required_files** +* **common_paths** * **alert_log_messages** * **imported_files** @@ -103,7 +105,7 @@ Example section **input_datasets** unzipped_path_containing_subject_dirs: "freesurfer" path_in_babs: inputs/data/freesurfer -This example shows two input datasets: +This example shows two input datasets: one is a raw BIDS dataset, and the other is a zipped FreeSurfer results from another BABS project. Previously, the commandline to use something like this would have required:: @@ -773,4 +775,76 @@ Notes: .. _required_files: +Section ``required_files`` +========================== + +.. note:: + + ``required_files`` is currently not fully implemented. + The field is accepted in the YAML file but filtering is not yet applied. + +.. _bids-inheritance: + +BIDS metadata inheritance (automatic) +===================================== + +For every non-zipped input dataset, BABS automatically stages the BIDS +*inherited* metadata each job needs, in addition to the per-subject (and +per-session) path. Under the `BIDS Inheritance Principle +`_, +a metadata file that sits *above* a data file in the directory hierarchy applies +to it. A per-subject (or per-session) sparse-checkout would otherwise drop those +upper-tier files — e.g. a top-level ``task-rest_bold.json`` carrying +``RepetitionTime``/``PhaseEncodingDirection``, ``participants.tsv``, or +``dataset_description.json`` — making BIDS Apps fail on otherwise-valid data. + +For each job, BABS includes every metadata file at the tiers *above* the job's +checkout: + +* the **dataset root** — always; +* the **subject tier** (``sub-XX/``) — additionally for ``session``-level jobs, + whose ``sub-XX/ses-YY`` checkout would otherwise miss files sitting directly + under ``sub-XX/`` (subject-level jobs already check out the whole ``sub-XX/`` + subtree). + +This is automatic and needs no configuration: ``dataset_description.json`` and +other inherited sidecars are staged out of the box. + +.. _common-paths: + +Section ``common_paths`` +========================= + +``common_paths`` is an optional escape hatch for **extra, non-inherited** files +that the automatic inheritance grab does not reach — for example a specific file +in a subdirectory, such as a shared ``sourcedata/NIDM/nidm.ttl``. It lists paths +relative to an input dataset's root; each is added to the job's sparse-checkout, +retrieved with ``datalad get -n``, and recorded as a ``datalad run`` input. + +It defaults to ``[]`` — BIDS inheritance already covers the dataset-level +sidecars, so valid BIDS needs nothing extra here. It is nested under the +relevant input dataset entry inside the ``input_datasets`` section. + +Example — also stage a specific shared file the inheritance grab won't reach: + +.. code-block:: yaml + + input_datasets: + BIDS: + is_zipped: false + origin_url: "/path/to/BIDS" + path_in_babs: inputs/data/BIDS + common_paths: + - "sourcedata/NIDM/nidm.ttl" + +Notes: + +* Paths are relative to the input dataset root (e.g., ``"sourcedata/NIDM/nidm.ttl"`` + not ``"inputs/data/BIDS/sourcedata/NIDM/nidm.ttl"``). +* Each path is retrieved individually with ``datalad get -n`` so you can track + exactly which files are fetched in the job log. +* This field has no effect on zipped input datasets. +* BIDS metadata inheritance is automatic (see above) — you do **not** need to + list ``dataset_description.json`` or other inherited sidecars here. + diff --git a/tests/test_generate_submit_script.py b/tests/test_generate_submit_script.py index 215d43cb..95929ba8 100644 --- a/tests/test_generate_submit_script.py +++ b/tests/test_generate_submit_script.py @@ -14,6 +14,7 @@ 'path_in_babs': 'inputs/data/BIDS', 'unzipped_path_containing_subject_dirs': 'inputs/data/BIDS', 'is_zipped': False, + 'common_paths': ['dataset_description.json'], }, ] @@ -29,6 +30,7 @@ 'path_in_babs': 'inputs/data/BIDS', 'unzipped_path_containing_subject_dirs': 'inputs/data/BIDS', 'is_zipped': False, + 'common_paths': [], }, ] @@ -176,3 +178,62 @@ def test_generate_submit_script_pipeline(tmp_path): if not passed: print(script_content) assert passed, status + + +def _render(input_datasets, processing_level): + """Render the participant job for a minimal single-app config (no YAML needed).""" + return generate_submit_script( + queue_system='slurm', + cluster_resources_config={'interpreting_shell': '/bin/bash'}, + script_preamble='', + job_scratch_directory='/tmp/job', + input_datasets=input_datasets, + processing_level=processing_level, + container_name='fmriprep', + zip_foldernames={'fmriprep': '0'}, + container_images=['containers/fmriprep.sif'], + analysis_path='/tmp/babs_project/analysis', + ) + + +def test_bids_inheritance_tiers_per_processing_level(): + """Root tier is grabbed for every job; the subject tier only for session jobs. + + These assert on the *generated script text* (like the rest of this module); + the runtime ``resolve_tier`` behavior is exercised by the e2e walkthrough. + """ + path = input_datasets_prep[0]['path_in_babs'] + subj = _render(input_datasets_prep, 'subject') + sess = _render(input_datasets_prep, 'session') + + # root tier resolved for both levels + assert f'resolve_tier "{path}" ""' in subj + assert f'resolve_tier "{path}" ""' in sess + + # subject tier resolved ONLY for session-level jobs (the gap session checkout misses) + assert f'resolve_tier "{path}" "${{subid}}"' in sess + assert f'resolve_tier "{path}" "${{subid}}"' not in subj + + # resolved paths are wired into `datalad run` as inputs + for script in (subj, sess): + assert 'DATALAD_INPUTS=()' in script + assert '${DATALAD_INPUTS[@]+"${DATALAD_INPUTS[@]}"}' in script + + +def test_bids_inheritance_skips_zipped_inputs(): + """Zipped inputs get no inheritance grab; only the unzipped one is resolved.""" + script = _render(input_datasets_fmriprep_ingressed_anat, 'subject') + # the unzipped BIDS dataset is resolved ... + assert 'resolve_tier "inputs/data/BIDS" ""' in script + # ... but the zipped freesurfer dataset (path_in_babs 'inputs/data') is not + assert 'resolve_tier "inputs/data" ""' not in script + + +def test_common_paths_threaded_into_consumers(): + """An explicit common_paths entry reaches get, sparse-checkout, and `datalad run -i`.""" + dss = [dict(input_datasets_prep[0], common_paths=['phenotype/participants.tsv'])] + script = _render(dss, 'subject') + full = 'inputs/data/BIDS/phenotype/participants.tsv' + assert f'datalad get -n "{full}"' in script # datalad get + assert "'phenotype/participants.tsv'" in script # sparse=( ... ) set + assert f'-i "{full}"' in script # datalad run input diff --git a/tests/test_input_datasets.py b/tests/test_input_datasets.py index dfa9c47a..e901a186 100644 --- a/tests/test_input_datasets.py +++ b/tests/test_input_datasets.py @@ -5,6 +5,30 @@ from babs.input_datasets import InputDatasets +def _bids(**overrides): + kwargs = { + 'name': 'BIDS', + 'origin_url': '/does/not/matter', + 'path_in_babs': 'inputs/data/BIDS', + 'is_zipped': False, + 'processing_level': 'subject', + } + kwargs.update(overrides) + return InputDataset(**kwargs) + + +def test_common_paths_default_is_empty(): + """common_paths defaults to [] (BIDS inheritance is automatic, not via this field).""" + # default: no common_paths supplied + assert _bids().common_paths == [] + # an explicit list is preserved as-is ... + assert _bids(common_paths=['phenotype/participants.tsv']).common_paths == [ + 'phenotype/participants.tsv' + ] + # ... including an explicit empty list + assert _bids(common_paths=[]).common_paths == [] + + @pytest.mark.parametrize( ('session_type', 'processing_level'), [