Skip to content

Commit dbfbb65

Browse files
authored
Drop the run-level cell count and read acquisitions from the registry (#32)
* Record the real cell count and finish time for packaging build_packaging_process was called before package_cells with cell_count=0, and the record was embedded verbatim in every cell, so every processing.json claimed zero cells packaged. Its end_date_time was also stamped before any packaging work ran. package_cells now resolves every cell before writing any, then builds the record from the resulting count and the actual finish time. It takes a factory rather than a record because neither value is known at the call site. * Drop the run-level cell count and read acquisitions from the registry Every asset is a single cell, so recording how many cells the run produced says nothing about the asset it sits in. build_packaging_process no longer takes cell_count and package_cells' factory needs only the finish time, which remains necessary so the record does not claim to have finished before the work ran. The acquisition now resolves DocDB v2 -> v1 -> S3, matching default_sources, with a staged copy only as a last resort. The registry holds what the indexer published; a mounted bundle may carry a stale copy or none. check_orientation reads the axes from it, so a wrong one mis-registers every cell without failing. Both fetchers are generalised to any core field. * Stop recording packaging as a processing step Packaging hardlinks reconstructions and writes metadata; the SWCs that come out are byte-identical to the ones that go in. A DataProcess for it describes assembling the asset rather than producing the data, in a document meant for the latter. Where the parent metadata came from stays on DataDescription.tags, which is indexed and so searchable, unlike output_parameters. Removing the step also removes the need for the finish-time factory, so package_cells returns to a single pass. --------- Co-authored-by: Peter Grotz <199680633+peter-grotz@users.noreply.github.com>
1 parent 638a938 commit dbfbb65

5 files changed

Lines changed: 297 additions & 153 deletions

File tree

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
"""Obtain a dataset's ``acquisition`` from the registry, falling back to S3.
2+
3+
The transform needs the acquisition's axis orientation, and ``RegistrationPipeline``
4+
takes it as a file path. Where that file comes from matters: a dataset's mounted bundle
5+
may carry a stale copy, or none, while the registry holds the record the indexer
6+
published.
7+
8+
The order matches :func:`~exaspim_swc_processing.sources.default_sources` -- DocDB v2,
9+
then v1, then the object in S3 -- so the registry is authoritative and S3 answers only
10+
for assets it has not indexed.
11+
"""
12+
13+
from __future__ import annotations
14+
15+
import json
16+
import logging
17+
from collections.abc import Sequence
18+
from pathlib import Path
19+
20+
from exaspim_swc_processing.parent_metadata import MetadataSource
21+
from exaspim_swc_processing.sources import (
22+
DOCDB_HOST,
23+
OPEN_DATA_BUCKET,
24+
Fetcher,
25+
docdb_fetcher,
26+
s3_fetcher,
27+
)
28+
29+
logger = logging.getLogger(__name__)
30+
31+
ACQUISITION_FIELD = "acquisition"
32+
"""Top-level field of a DocDB record holding the acquisition."""
33+
34+
ACQUISITION_KEY = "acquisition.json"
35+
"""Object at the asset root holding the acquisition."""
36+
37+
38+
class AcquisitionNotFoundError(LookupError):
39+
"""Raised when no source holds an acquisition for the dataset."""
40+
41+
42+
def acquisition_sources(
43+
host: str = DOCDB_HOST, bucket: str = OPEN_DATA_BUCKET
44+
) -> list[tuple[MetadataSource, Fetcher]]:
45+
"""Build the resolution order for an acquisition.
46+
47+
Parameters
48+
----------
49+
host : str, optional
50+
DocDB host, by default :data:`~exaspim_swc_processing.sources.DOCDB_HOST`.
51+
bucket : str, optional
52+
S3 bucket, by default :data:`~exaspim_swc_processing.sources.OPEN_DATA_BUCKET`.
53+
54+
Returns
55+
-------
56+
list[tuple[MetadataSource, Fetcher]]
57+
Sources in priority order: DocDB v2, DocDB v1, then S3.
58+
"""
59+
return [
60+
(MetadataSource.DOCDB_V2, docdb_fetcher("v2", host, field=ACQUISITION_FIELD)),
61+
(MetadataSource.DOCDB_V1, docdb_fetcher("v1", host, field=ACQUISITION_FIELD)),
62+
(MetadataSource.S3, s3_fetcher(bucket, filename=ACQUISITION_KEY)),
63+
]
64+
65+
66+
def resolve_acquisition(
67+
asset_name: str,
68+
destination: Path,
69+
sources: Sequence[tuple[MetadataSource, Fetcher]] | None = None,
70+
local_fallback: str = "",
71+
) -> tuple[Path, MetadataSource | None]:
72+
"""Write the dataset's acquisition to a file and say where it came from.
73+
74+
Parameters
75+
----------
76+
asset_name : str
77+
Processed dataset name.
78+
destination : Path
79+
File to write the acquisition to when it comes from a registry or S3.
80+
sources : Sequence[tuple[MetadataSource, Fetcher]] | None, optional
81+
Sources in priority order. Defaults to :func:`acquisition_sources`.
82+
local_fallback : str, optional
83+
An acquisition already on disk, used only when no source answers.
84+
85+
Returns
86+
-------
87+
tuple[Path, MetadataSource | None]
88+
The file to read, and the source it came from -- ``None`` when the local
89+
fallback was used.
90+
91+
Raises
92+
------
93+
AcquisitionNotFoundError
94+
If no source answers and there is no usable local fallback.
95+
"""
96+
for source, fetch in sources if sources is not None else acquisition_sources():
97+
try:
98+
document = fetch(asset_name)
99+
except Exception as error: # noqa: BLE001 - an unreachable source is not fatal
100+
logger.info("Acquisition source %s failed: %s", source.value, error)
101+
continue
102+
if not document:
103+
continue
104+
destination.parent.mkdir(parents=True, exist_ok=True)
105+
destination.write_text(json.dumps(document, indent=2), encoding="utf-8")
106+
logger.info("Acquisition for %s read from %s", asset_name, source.value)
107+
return destination, source
108+
109+
if local_fallback and Path(local_fallback).is_file():
110+
logger.warning(
111+
"No acquisition for %s in DocDB or S3; using the copy staged at %s",
112+
asset_name,
113+
local_fallback,
114+
)
115+
return Path(local_fallback), None
116+
raise AcquisitionNotFoundError(
117+
f"No acquisition for {asset_name} in DocDB v2, DocDB v1 or S3, and no local copy"
118+
)

‎src/exaspim_swc_processing/packaging.py‎

Lines changed: 8 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,12 @@
1212
requirement that the upstream assets cannot currently satisfy, so the asset would advertise
1313
itself as incomplete. Quality control belongs to a later stage and is not written here.
1414
15-
The packaging step records itself as a :class:`~aind_data_schema.core.processing.DataProcess`
16-
appended to each cell's record. It is a real processing step — it is what produced the
17-
directory — and it carries the provenance of where the parent metadata was read from.
15+
Packaging does not record itself as a
16+
:class:`~aind_data_schema.core.processing.DataProcess`. It changes no data -- the
17+
reconstructions are hardlinked, byte for byte -- so a record of it would describe
18+
assembling the asset rather than producing the data, in a document meant for the latter.
19+
Where the parent metadata was read from is carried on ``DataDescription.tags``, which is
20+
indexed and therefore searchable, unlike ``output_parameters``.
1821
"""
1922

2023
from __future__ import annotations
@@ -26,8 +29,7 @@
2629
from pathlib import Path
2730

2831
from aind_data_schema.components.identifiers import Code
29-
from aind_data_schema.core.processing import DataProcess, ProcessStage
30-
from aind_data_schema_models.process_names import ProcessName
32+
from aind_data_schema.core.processing import DataProcess
3133

3234
from exaspim_swc_processing.data_description import (
3335
DataDescriptionDerivationError,
@@ -41,12 +43,6 @@
4143

4244
logger = logging.getLogger(__name__)
4345

44-
PACKAGING_STEP_NAME = "exaspim_swc_packaging"
45-
"""``DataProcess.name`` of the step this module performs."""
46-
47-
DEFAULT_EXPERIMENTERS = ("Peter Grotz",)
48-
"""Default ``DataProcess.experimenters`` for the packaging step."""
49-
5046
DATA_DESCRIPTION_FILENAME = "data_description.json"
5147
PROCESSING_FILENAME = "processing.json"
5248

@@ -103,65 +99,13 @@ class PackagingResult:
10399
skipped: tuple[SkippedCell, ...]
104100

105101

106-
def build_packaging_process(
107-
parent: ParentMetadata,
108-
code: Code,
109-
start_time: datetime,
110-
end_time: datetime,
111-
output_path: str,
112-
cell_count: int,
113-
experimenters: Sequence[str] = DEFAULT_EXPERIMENTERS,
114-
) -> DataProcess:
115-
"""Describe the packaging step itself.
116-
117-
Parameters
118-
----------
119-
parent : ParentMetadata
120-
Resolved parent metadata, supplying the provenance of where it was read from.
121-
code : Code
122-
The packaging code: repository URL, name and version.
123-
start_time : datetime
124-
When packaging started.
125-
end_time : datetime
126-
When packaging finished.
127-
output_path : str
128-
Where the cell directories were written, relative to ``/results``.
129-
cell_count : int
130-
Number of cells packaged in the run.
131-
experimenters : Sequence[str], optional
132-
Who is responsible for the run, by default :data:`DEFAULT_EXPERIMENTERS`.
133-
134-
Returns
135-
-------
136-
DataProcess
137-
The step record, carrying :meth:`ParentMetadata.provenance` in
138-
``output_parameters``. Typed ``OTHER`` because regrouping files into per-cell
139-
assets is not one of the vocabulary's defined operations; the docs direct
140-
unmatched operations to ``ANALYSIS`` or ``OTHER``, and ``OTHER`` requires the
141-
``name`` and ``notes`` set here.
142-
"""
143-
return DataProcess(
144-
process_type=ProcessName.OTHER,
145-
name=PACKAGING_STEP_NAME,
146-
stage=ProcessStage.PROCESSING,
147-
code=code,
148-
experimenters=list(experimenters),
149-
start_date_time=start_time,
150-
end_date_time=end_time,
151-
output_path=output_path,
152-
output_parameters={**parent.provenance(), "cells_packaged": cell_count},
153-
notes="Regrouped pipeline outputs into one derived asset per reconstruction.",
154-
)
155-
156-
157102
def package_cells(
158103
stage_root: Path,
159104
output_root: Path,
160105
parent: ParentMetadata,
161106
stage_processes: Sequence[DataProcess],
162107
pipeline: Code,
163108
creation_time: datetime,
164-
packaging_process: DataProcess,
165109
specs: Sequence[ArtifactSpec] = ARTIFACT_SPECS,
166110
overrides: Mapping[str, object] | None = None,
167111
) -> PackagingResult:
@@ -181,8 +125,6 @@ def package_cells(
181125
The pipeline that produced the run.
182126
creation_time : datetime
183127
Creation time shared by every cell in the run.
184-
packaging_process : DataProcess
185-
Record of this packaging step, appended to each cell's processing record.
186128
specs : Sequence[ArtifactSpec], optional
187129
Artifacts to collect, by default :data:`~exaspim_swc_processing.layout.ARTIFACT_SPECS`.
188130
overrides : Mapping[str, object] | None, optional
@@ -195,7 +137,7 @@ def package_cells(
195137
skipped rather than written without metadata.
196138
"""
197139
specs = tuple(specs)
198-
processes = [*stage_processes, packaging_process]
140+
record = build_cell_processing(list(stage_processes), pipeline)
199141
packaged: list[PackagedCell] = []
200142
skipped: list[SkippedCell] = []
201143

@@ -222,7 +164,6 @@ def package_cells(
222164

223165
cell_dir = output_root / description.name
224166
_write_cell_directory(cell, cell_dir, specs)
225-
record = build_cell_processing(processes, pipeline)
226167
(cell_dir / DATA_DESCRIPTION_FILENAME).write_text(
227168
description.model_dump_json(indent=2), encoding="utf-8"
228169
)

‎src/exaspim_swc_processing/sources.py‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,10 @@ def _docdb_client(version: str, host: str) -> DocDbClient:
9696

9797

9898
def docdb_fetcher(
99-
version: str, host: str = DOCDB_HOST, client: DocDbClient | None = None
99+
version: str,
100+
host: str = DOCDB_HOST,
101+
client: DocDbClient | None = None,
102+
field: str = CORE_FIELD,
100103
) -> Fetcher:
101104
"""Build a fetcher reading one DocDB endpoint.
102105
@@ -112,12 +115,15 @@ def docdb_fetcher(
112115
Host serving the index, by default :data:`DOCDB_HOST`.
113116
client : DocDbClient | None, optional
114117
A pre-built client. Constructed on first use when omitted.
118+
field : str, optional
119+
Core field to read, by default :data:`CORE_FIELD`. Any top-level field of the
120+
record works, e.g. ``"acquisition"``.
115121
116122
Returns
117123
-------
118124
Fetcher
119-
Returns the asset's raw ``data_description``, or ``None`` if the endpoint has no
120-
record for it.
125+
Returns the asset's raw document for ``field``, or ``None`` if the endpoint has
126+
no record for it.
121127
"""
122128

123129
def _fetch(asset_name: str) -> dict | None:
@@ -136,16 +142,14 @@ def _fetch(asset_name: str) -> dict | None:
136142
resolved = client if client is not None else _docdb_client(version, host)
137143
records = resolved.retrieve_docdb_records(
138144
filter_query={"name": asset_name},
139-
projection={CORE_FIELD: 1},
145+
projection={field: 1},
140146
limit=1,
141147
)
142148
if not records:
143149
return None
144-
document = records[0].get(CORE_FIELD)
150+
document = records[0].get(field)
145151
if not document:
146-
logger.warning(
147-
"DocDB %s has a record for %r with no %s", version, asset_name, CORE_FIELD
148-
)
152+
logger.warning("DocDB %s has a record for %r with no %s", version, asset_name, field)
149153
return None
150154
return document
151155

@@ -173,6 +177,7 @@ def _s3_client() -> S3Client:
173177
def s3_fetcher(
174178
bucket: str = OPEN_DATA_BUCKET,
175179
client: S3Client | None = None,
180+
filename: str = DATA_DESCRIPTION_KEY,
176181
) -> Fetcher:
177182
"""Build a fetcher reading the asset's ``data_description.json`` from S3.
178183
@@ -185,12 +190,14 @@ def s3_fetcher(
185190
Bucket holding the asset, by default :data:`OPEN_DATA_BUCKET`.
186191
client : S3Client | None, optional
187192
A pre-built S3 client. Constructed on first use when omitted.
193+
filename : str, optional
194+
Object to read from the asset root, by default :data:`DATA_DESCRIPTION_KEY`.
188195
189196
Returns
190197
-------
191198
Fetcher
192-
Returns the raw ``data_description``, or ``None`` if the object is absent or is
193-
not readable as JSON.
199+
Returns the raw document, or ``None`` if the object is absent or is not readable
200+
as JSON.
194201
"""
195202

196203
def _fetch(asset_name: str) -> dict | None:
@@ -207,12 +214,12 @@ def _fetch(asset_name: str) -> dict | None:
207214
The raw document, or ``None`` if absent or unreadable.
208215
"""
209216
resolved = client if client is not None else _s3_client()
210-
key = f"{asset_name}/{DATA_DESCRIPTION_KEY}"
217+
key = f"{asset_name}/{filename}"
211218
try:
212219
response = resolved.get_object(Bucket=bucket, Key=key)
213220
return json.loads(response["Body"].read())
214221
except Exception as error: # noqa: BLE001 - any read failure means "not here"
215-
logger.info("No %s in s3://%s/%s (%s)", DATA_DESCRIPTION_KEY, bucket, key, error)
222+
logger.info("No %s in s3://%s/%s (%s)", filename, bucket, key, error)
216223
return None
217224

218225
return _fetch

0 commit comments

Comments
 (0)