Details
-
Type:
RFC
-
Status: Implemented
-
Resolution: Done
-
Component/s: DM
-
Labels:None
Description
The exposureId value in visitInfo is not a visit-exposure level identifier, but rather a ccd-exposure level identifier (see docstring for obs.base.CameraMapper._computeCcdExposureId() for the only place where this seems to be specified). Thus, it should not be included in visitInfo, which is only things that are common across the visit.
We do need a visit-exposure level identifier to be included in visitInfo, so that we can link exposures within visits without using the butler (e.g. without dataRef.dataId['visit']). This RFC likely requires that makeRawVisitInfo() adds the butler visitId to the new visitInfo.visitId, unless there is some other way to access the visitId value besides the above butler call.
Concrete proposal:
- Move exposureId from VisitInfo into ExposureInfo.
- Add visitId to VisitInfo, and include it as part of makeRawVisitInfo.
- Document the meaning of exposureId to be clear that it is per-ccd.
- Document whether visitId can be derived from exposureId, or whether those are decoupled.
- Document the meaning of visitId to be clear that it is per-visit.
See this discussion that generated this RFC:
https://lsstc.slack.com/archives/C3UCAEW3D/p1517724549000034
Attachments
Issue Links
- is triggering
-
DM-13738 Remove exposureId from visitInfo
- Invalid
-
DM-13942 Remove exposureId and from VisitInfo and add visitId
- Invalid
-
- Done
-
- Done
-
- Done
- relates to
-
- Implemented
-
- To Do
- mentioned in
-
Page Loading...
Activity
I agree that exposureId should be removed from VisitInfo, and ultimately replaced by an ID corresponding to an actual Visit.
I am ambivalent about the priority of doing so, and am a bit skeptical that defining a meaningful Visit ID is worth trying to achieve before Gen3.
A side note: the current schema for Gen3's DataUnits is that Exposure and Visit both exist, but Exposure is not the combination of a Visit and a Sensor. It is a single observation with a camera (maybe a snap of a raw science visit, maybe a raw calibration image - the latter are not associated into Visits). We could revisit this, but I think it makes perfect sense if you haven't been corrupted by "exposure" to the afw.image.Exposure class. The set of people who have been thus corrupted is not even as large as the set of people working for LSST DM, while the set of people who will see these DataUnits should eventually be much larger. I think we probably cannot live in the long term with Exposure meaning "Visit + Sensor". My RFC to rename afw.image.Exposure is blocked "only" by the fact that I have no idea what to call it instead.
Jim Bosch your comment suggests that there should be two implementation tickets for this: the first one to move ExposureId out of VisitInfo (which can and should be done now, to prevent API breakage in the future), the second to add VisitId to VisitInfo (which should maybe wait for Gen3 Butler).
> My RFC to rename afw.image.Exposure is blocked "only" by the fact that I have no idea what to call it instead.
SensorExposure or ChipExposure?
In which case, the current ExposureId would become SensorExposureId, I guess?
the first one to move ExposureId out of VisitInfo (which can and should be done now, to prevent API breakage in the future), the second to add VisitId to VisitInfo (which should maybe wait for Gen3 Butler).
Sounds reasonable.
SensorExposure or ChipExposure?
Note that we also use this class for coadded images, which is the big reason I dislike the name.
In which case, the current ExposureId would become SensorExposureId, I guess?
For raw (science or calibration) images, yes. For processed images after we've combined snaps, it'd be a VisitSensorId. It might make more sense to just have a word for the integer ID formed by concatenating the set of DataUnits used to identify that DatasetType (so you don't need a new name for e.g. Jointcal outputs that also need to encode a Tract). There's no guarantee that stays below 64 bits in general, of course.
I often want to grab a set of exposures by specifying the visit they all belong to. The proposed change sounds great to me, so long as care is taken to make sure it doesn't break any common use cases of various obs_packages.
I have no strong opinions about renaming exposure itself, except to note that attaching words like "visit," "sensor," "ccd," and/or "chip" to "exposure" doesn't inherently add clarity about what the object in question is.
John Parejko are you ready to adopt this RFC?
Adopted, with implementation ticket as a two story epic: DM-13942. The details raised by Russell Owen and Jim Bosch can be decided on as those tickets are implemented.
I'm going to leave the names as-is here: future name improvements per Meredith Rawls's comment can be done in other RFCs.
This sounds like a great idea to me. This fixes a long-standing issue in a very natural way.
I anticipate it will take some obs-package-specific work to set the exposureId field in ExposureInfo and the visitId field in VisitInfo (especially since we presently have no standard way to compute a visit ID from an exposure ID), but it is work that I feel is well worth doing.