Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DM-44605: Prompt processing can do ISR for LATISS calibrations #181

Merged
merged 3 commits into from
Aug 14, 2024
Merged

DM-44605: Prompt processing can do ISR for LATISS calibrations #181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fafb07a
Do not constrain visit_system in data query
hsinfang Jun 11, 2024
6bd89f8
Update reference to ts_idl
hsinfang Aug 12, 2024
e706255
Skip preloading spatial datasets with dubious coordinates
hsinfang Aug 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 22 additions & 15 deletions python/activator/middleware_interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,12 +283,6 @@ class MiddlewareInterface:
constructing URIs to retrieve incoming files. The default is
appropriate for use in the USDF environment; typically only
change this when running local tests.

Raises
------
ValueError
Raised if ``visit`` does not have equatorial coordinates and sky
rotation angles.
"""
DATASET_IDENTIFIER = "Live"
"""The dataset ID used for Sasquatch uploads.
Expand Down Expand Up @@ -327,12 +321,22 @@ def __init__(self, central_butler: Butler, image_bucket: str, visit: FannedOutVi
skymap: str, local_repo: str, local_cache: DatasetCache,
prefix: str = "s3://"):
self.visit = visit
# Usually prompt processing only cares about on-sky images and expects all of
# them to have equatorial coordinates and sky rotation angles. In some cases,
# ISR can be done but sensible results from further pipelines cannot be expected.
# Use self._skip_spatial_preload to indicate dubious coordinates and to skip
# preloading spatial datasets, but continue the processing.
self._skip_spatial_preload = False
if self.visit.coordinateSystem != FannedOutVisit.CoordSys.ICRS:
raise ValueError("Only ICRS coordinates are supported in Visit, "
f"got {self.visit.coordinateSystem!r} instead.")
self._skip_spatial_preload = True
_log.error("Only ICRS coordinates are fully supported. "
f"Got {self.visit.coordinateSystem!r} instead in {self.visit}. "
"Spatial datasets won't be loaded.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be a warning, given that we are plowing ahead anyway?

Copy link
Collaborator Author

@hsinfang hsinfang Aug 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I slightly prefer this to be an error for now, so (1) it does not drown in so many warnings from the pipelines, and (2) in a way it is an error as the system isn't really designed to process those visits and something may go wrong later. I feel that this is similar to the level of finding no templates for some visits but we continue with the next best thing.

if self.visit.rotationSystem != FannedOutVisit.RotSys.SKY:
raise ValueError("Only sky camera rotations are supported in Visit, "
f"got {self.visit.rotationSystem!r} instead.")
self._skip_spatial_preload = True
_log.error("Only sky camera rotations are fully supported. "
f"Got {self.visit.rotationSystem!r} instead in {self.visit}. "
"Spatial datasets won't be loaded.")

# Deployment/version ID -- potentially expensive to generate.
self._deployment = self._get_deployment()
Expand Down Expand Up @@ -441,7 +445,7 @@ def _init_visit_definer(self):

``self._init_local_butler`` must have already been run.
"""
define_visits_config = lsst.obs.base.DefineVisitsConfig()
define_visits_config = lsst.obs.base.DefineVisitsConfig(groupExposures="one-to-one")
self.define_visits = lsst.obs.base.DefineVisitsTask(config=define_visits_config, butler=self.butler)

def _define_dimensions(self):
Expand Down Expand Up @@ -535,7 +539,8 @@ def prep_butler(self) -> None:
with lsst.utils.timer.time_this(_log, msg="prep_butler", level=logging.DEBUG):
_log.info(f"Preparing Butler for visit {self.visit!r}")

self._write_region_time() # Must be done before preprocessing pipeline
if not self._skip_spatial_preload:
self._write_region_time() # Must be done before preprocessing pipeline

# repos may have been modified by other MWI instances.
# TODO: get a proper synchronization API for Butler
Expand Down Expand Up @@ -583,6 +588,11 @@ def _find_data_to_preload(self):
calibs : set [`~lsst.daf.butler.DatasetRef`]
The subset of ``datasets`` representing calibs.
"""
with lsst.utils.timer.time_this(_log, msg="prep_butler (find calibs)", level=logging.DEBUG):
calib_datasets = set(self._export_calibs(self.visit.detector, self.visit.filters))
if self._skip_spatial_preload:
return (calib_datasets, calib_datasets)

detector = self.camera[self.visit.detector]
wcs = self._predict_wcs(detector)
center, radius = self._detector_bounding_circle(detector, wcs)
Expand All @@ -592,8 +602,6 @@ def _find_data_to_preload(self):
with lsst.utils.timer.time_this(_log, msg="prep_butler (find templates)", level=logging.DEBUG):
template_datasets = set(self._export_skymap_and_templates(
center, detector, wcs, self.visit.filters))
with lsst.utils.timer.time_this(_log, msg="prep_butler (find calibs)", level=logging.DEBUG):
calib_datasets = set(self._export_calibs(self.visit.detector, self.visit.filters))
with lsst.utils.timer.time_this(_log, msg="prep_butler (find ML models)", level=logging.DEBUG):
model_datasets = set(self._export_ml_models())
return (refcat_datasets | template_datasets | calib_datasets | model_datasets,
Expand Down Expand Up @@ -1386,7 +1394,6 @@ def run_pipeline(self, exposure_ids: set[int]) -> None:
where = (
f"instrument='{self.visit.instrument}' and detector={self.visit.detector}"
f" and exposure in ({','.join(str(x) for x in exposure_ids)})"
" and visit_system = 0"
)
preload_run = self._get_preload_run(self._day_obs)
init_pre_runs = [self._get_init_output_run(f, self._day_obs) for f in self._get_pre_pipeline_files()]
Expand Down
6 changes: 3 additions & 3 deletions python/activator/visit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,22 @@ class BareVisit:
# Inherited from SAL next_visit schema; keep in sync with
# https://ts-xml.lsst.io/sal_interfaces/ScriptQueue.html#nextvisit
class CoordSys(enum.IntEnum):
# This is a redeclaration of lsst.ts.idl.enums.Script.MetadataCoordSys,
# This is a redeclaration of lsst.ts.xml.enums.Script.MetadataCoordSys,
# but we need BareVisit to work in code that can't import lsst.ts.
NONE = 1
ICRS = 2
OBSERVED = 3
MOUNT = 4

class RotSys(enum.IntEnum):
# Redeclaration of lsst.ts.idl.enums.Script.MetadataRotSys.
# Redeclaration of lsst.ts.xml.enums.Script.MetadataRotSys.
NONE = 1
SKY = 2
HORIZON = 3
MOUNT = 4

class Dome(enum.IntEnum):
# Redeclaration of lsst.ts.idl.enums.Script.MetadataDome.
# Redeclaration of lsst.ts.xml.enums.Script.MetadataDome.
CLOSED = 1
OPEN = 2
EITHER = 3
Expand Down
Loading