Details
-
Type:
RFC
-
Status: Adopted
-
Resolution: Done
-
Component/s: DM
-
Labels:None
Description
DM-5502 details metadata that should be added to ExposureInfo. This RFC gives the details. I am using a short duration because DM-5502 already is basically an RFC.
- Add set/getBBox methods for the bounding box of the exposure, an afw::geom::Box2I (not a shared pointer, thus matching exposure.getBBox()). This will be automatically updated when one retrieves a subexposure or calls exposure.setXY0 but will not be safe against changing the xy0 of the underlying masked image (just as one can, but should not, manually change the xy0 of the variance plane or mask in a masked image).
- Add set/getField methods for data describing the center of the field of view; a shared_ptr<Field>, where Field is a new struct with attributes:
- exposureId: exposure ID, a long int; this needs a demangler to turn it into a visit ID (see DM-6912)
- sky: coordinates of the center of the field of view; an afw::coord::IcrsCoord
- observed: refracted, topocentric Az/Alt; an afw::coord::TopocentricCoord
- airmass, a double
- midDate, a daf::base::DateTime
- last local apparent sidereal time, an afw::geom::Angle
- positionAngle, angle from celestial north to x axis of focal plane, an afw::geom::Angle
- airTemperature in C, a double
- airPressure in Pascals, a double
- humidity as a fraction, a double
- observatory longitude, latitude and elevation of telescope; a afw::coord::Observatory
- note that exposure time will remain in Calibrate, but Field documentation will say so
- Add getAirmass(pixPos) and getObserved(pixPos) functions that use Field, Wcs and Detector; pixPos will default to the center of the detector.
One simplified alternative is to store the airmass, position angle and observed position for the center of the CCD, rather than the center of the field, and ditch the functions that compute these values at other points on the CCD. If we do that, the name of that object will need some thought, since it must be made clear that the data is for the center of the CCD, not the center of the field of view. My proposal matches my expectation that most observatories will provide the data for the center of the field of view, and that most users will be content with that, but some algorithms, such as DCR correction, may require those values at the center of the CCD or even at different points across the CCD.
In any case we have to be careful how IsrTask loads or computes this data, since one can imagine that an observatory might provide airmass, etc. at the center of each CCD instead of at the center of the field.
Attachments
Issue Links
Activity
| Description |
- Add {{set/getBBox}} methods for the bounding box of the exposure, an {{afw::geom::Box2I}} (not a shared pointer, thus matching exposure.getBBox()). This will be automatically updated when one retrieves a subexposure or calls exposure.setXY0 but will not be safe against changing the xy0 of the underlying masked image (just as one can, but should not, manually change the xy0 of the variance plane or mask in a masked image). - Add {{set/getField}} methods for data describing the center of the field of view; a {{shared_ptr<Field>}}, where {{Field}} is a new struct with attributes: - {{exposureId}}: exposure ID, a {{long unsigned int}}? - {{sky}}: coordinates of the center of the field of view; an {{afw::coord::IcrsCoord}} - {{observed}}: refracted, topocentric Az/Alt; an {{afw::coord::TopocentricCoord}} - {{airmass}}, a {{double}} - {{midDate}}, a {{daf::base::DateTime}} - {{last}} local apparent sidereal time, an {{afw::geom::Angle}} - {{positionAngle}}, an {{afw::geom::Angle}} - {{airTemperature}} in C, a {{double}} - {{airPressure}} in Pascals, a {{double}} - {{humidity}} as a fraction, a {{double}} - {{observatory}} longitude, latitude and elevation of telescope; a {{afw::coord::Observatory}} - note that exposure time will remain in {{Calibrate}}, but {{Field}} documentation will say so - Add {{getAirmass(pixPos)}} and {{getObserved(pixPos)}} functions that use {{Field}}, {{Wcs}} and {{Detector}}; {{pixPos}} will default to the center of the detector. One simplified alternative is to store the airmass, position angle and observed position for the center of the CCD, rather than the center of the field, and ditch the functions that compute these values at other points on the CCD. If we do that, the name of that object will need some thought, since it must be made clear that the data is for the center of the CCD, not the center of the field of view. My proposal matches my expectation that most observatories will provide the data for the center of the field of view, and that most users will be content with that, but some algorithms, such as DCR correction, may require those values at the center of the CCD or even at different points across the CCD. In any case we have to be careful how {{IsrTask}} loads or computes this data, since one can imagine that an observatory might provide airmass, etc. at the center of each CCD instead of at the center of the field. |
- Add {{set/getBBox}} methods for the bounding box of the exposure, an {{afw::geom::Box2I}} (not a shared pointer, thus matching exposure.getBBox()). This will be automatically updated when one retrieves a subexposure or calls exposure.setXY0 but will not be safe against changing the xy0 of the underlying masked image (just as one can, but should not, manually change the xy0 of the variance plane or mask in a masked image). - Add {{set/getField}} methods for data describing the center of the field of view; a {{shared_ptr<Field>}}, where {{Field}} is a new struct with attributes: - {{exposureId}}: exposure ID, a {{long unsigned int}}? - {{sky}}: coordinates of the center of the field of view; an {{afw::coord::IcrsCoord}} - {{observed}}: refracted, topocentric Az/Alt; an {{afw::coord::TopocentricCoord}} - {{airmass}}, a {{double}} - {{midDate}}, a {{daf::base::DateTime}} - {{last}} local apparent sidereal time, an {{afw::geom::Angle}} - {{positionAngle}}, angle from celestial north to x axis of CCD, an {{afw::geom::Angle}} - {{airTemperature}} in C, a {{double}} - {{airPressure}} in Pascals, a {{double}} - {{humidity}} as a fraction, a {{double}} - {{observatory}} longitude, latitude and elevation of telescope; a {{afw::coord::Observatory}} - note that exposure time will remain in {{Calibrate}}, but {{Field}} documentation will say so - Add {{getAirmass(pixPos)}} and {{getObserved(pixPos)}} functions that use {{Field}}, {{Wcs}} and {{Detector}}; {{pixPos}} will default to the center of the detector. One simplified alternative is to store the airmass, position angle and observed position for the center of the CCD, rather than the center of the field, and ditch the functions that compute these values at other points on the CCD. If we do that, the name of that object will need some thought, since it must be made clear that the data is for the center of the CCD, not the center of the field of view. My proposal matches my expectation that most observatories will provide the data for the center of the field of view, and that most users will be content with that, but some algorithms, such as DCR correction, may require those values at the center of the CCD or even at different points across the CCD. In any case we have to be careful how {{IsrTask}} loads or computes this data, since one can imagine that an observatory might provide airmass, etc. at the center of each CCD instead of at the center of the field. |
- exposureId is mapper specific. This is probably OK, but should be called out (I just added a request for a demangler to DM-6912)
- I find "sky" a confusing name. How about "boresightSky"?
- I find "observed" confusing too. How about "boresightAltAz"
- Why is airmass included, instead of always calling getAirmass()?
- Is "last" an autocorrect for "lst"? Isn't it a derived quantity too given the time and alt/az? "getLST()" or "getLocalSiderealTime()" (possibly following the code standards)
At first glance this looks entirely reasonable; I'll try to do a more careful look later, but some preliminary (minor) quibbles:
note that exposure time will remain in Calib, but Field documentation will say so
This seems like an excellent opportunity to move it out of Calib into the new class.
but will not be safe against changing the xy0 of the underlying masked image
This is why I didn't add it to ExposureInfo originally, but I think I've come around to the idea that it's better to have it even if it's not totally safe. Thinking longer term, maybe this is an argument for making the bbox of all images immutable?
Instead of Field, maybe call this VisitInfo? I think Field is fairly heavily overloaded (not in our code, but in astronomy in general), and not quite descriptive enough for something that really only refers to exposure-level images. If we're willing to go a bit more extreme, I'd advocate calling this ExposureInfo, and using something other than Exposure for the combination of MaskedImage + metadata, since we use that class for coadds as well (and coadds are absolutely not exposures). But I don't have any great ideas for what our current Exposure should be called, and obviously that'd be a much more disruptive change than what you're proposing here. I guess I'm just saying it's unfortunate that the word "exposure" is taken by something isn't really (always) an exposure, and hence we're forced to consider words like "visit" or "field" instead when we really do mean "exposure".
I think last and maybe sky and positionAngle need more descriptive names.
To answer Robert Lupton's questions:
- I'll annotate exposureId. I'm not happy with a mangled value, but having a demangler will help.
- I don't feel that "boresight" is useful in the names because "Field" is already expressly meant to provide data for the center of the field.
- I agree "observed" is not fully obvious, but it attempts to make clear that refraction has been applied. Perhaps "observedAzAlt"?
- I am surprised about your complaint about airmass. in
DM-5502you pointed out that observatories can do a good job computing airmass and we should use it as provided. I just adopted that suggestion. Doing a thorough job requires more information than I think is worth carrying around in ExposureInfo (but I felt we could scale the value to other points in the field well enough to get by with what we have). - last is local apparent sideral time (LAST). It is subtly different than local mean sidereal time. I would rather be specific, and LAST is more useful than LMST. Unfortunately our case rules make it look like a word instead of an acronym.
| Description |
- Add {{set/getBBox}} methods for the bounding box of the exposure, an {{afw::geom::Box2I}} (not a shared pointer, thus matching exposure.getBBox()). This will be automatically updated when one retrieves a subexposure or calls exposure.setXY0 but will not be safe against changing the xy0 of the underlying masked image (just as one can, but should not, manually change the xy0 of the variance plane or mask in a masked image). - Add {{set/getField}} methods for data describing the center of the field of view; a {{shared_ptr<Field>}}, where {{Field}} is a new struct with attributes: - {{exposureId}}: exposure ID, a {{long unsigned int}}? - {{sky}}: coordinates of the center of the field of view; an {{afw::coord::IcrsCoord}} - {{observed}}: refracted, topocentric Az/Alt; an {{afw::coord::TopocentricCoord}} - {{airmass}}, a {{double}} - {{midDate}}, a {{daf::base::DateTime}} - {{last}} local apparent sidereal time, an {{afw::geom::Angle}} - {{positionAngle}}, angle from celestial north to x axis of CCD, an {{afw::geom::Angle}} - {{airTemperature}} in C, a {{double}} - {{airPressure}} in Pascals, a {{double}} - {{humidity}} as a fraction, a {{double}} - {{observatory}} longitude, latitude and elevation of telescope; a {{afw::coord::Observatory}} - note that exposure time will remain in {{Calibrate}}, but {{Field}} documentation will say so - Add {{getAirmass(pixPos)}} and {{getObserved(pixPos)}} functions that use {{Field}}, {{Wcs}} and {{Detector}}; {{pixPos}} will default to the center of the detector. One simplified alternative is to store the airmass, position angle and observed position for the center of the CCD, rather than the center of the field, and ditch the functions that compute these values at other points on the CCD. If we do that, the name of that object will need some thought, since it must be made clear that the data is for the center of the CCD, not the center of the field of view. My proposal matches my expectation that most observatories will provide the data for the center of the field of view, and that most users will be content with that, but some algorithms, such as DCR correction, may require those values at the center of the CCD or even at different points across the CCD. In any case we have to be careful how {{IsrTask}} loads or computes this data, since one can imagine that an observatory might provide airmass, etc. at the center of each CCD instead of at the center of the field. |
- Add {{set/getBBox}} methods for the bounding box of the exposure, an {{afw::geom::Box2I}} (not a shared pointer, thus matching exposure.getBBox()). This will be automatically updated when one retrieves a subexposure or calls exposure.setXY0 but will not be safe against changing the xy0 of the underlying masked image (just as one can, but should not, manually change the xy0 of the variance plane or mask in a masked image). - Add {{set/getField}} methods for data describing the center of the field of view; a {{shared_ptr<Field>}}, where {{Field}} is a new struct with attributes: - {{exposureId}}: exposure ID, a {{long int}}; this needs a demangler to turn it into a visit ID - {{sky}}: coordinates of the center of the field of view; an {{afw::coord::IcrsCoord}} - {{observed}}: refracted, topocentric Az/Alt; an {{afw::coord::TopocentricCoord}} - {{airmass}}, a {{double}} - {{midDate}}, a {{daf::base::DateTime}} - {{last}} local apparent sidereal time, an {{afw::geom::Angle}} - {{positionAngle}}, angle from celestial north to x axis of CCD, an {{afw::geom::Angle}} - {{airTemperature}} in C, a {{double}} - {{airPressure}} in Pascals, a {{double}} - {{humidity}} as a fraction, a {{double}} - {{observatory}} longitude, latitude and elevation of telescope; a {{afw::coord::Observatory}} - note that exposure time will remain in {{Calibrate}}, but {{Field}} documentation will say so - Add {{getAirmass(pixPos)}} and {{getObserved(pixPos)}} functions that use {{Field}}, {{Wcs}} and {{Detector}}; {{pixPos}} will default to the center of the detector. One simplified alternative is to store the airmass, position angle and observed position for the center of the CCD, rather than the center of the field, and ditch the functions that compute these values at other points on the CCD. If we do that, the name of that object will need some thought, since it must be made clear that the data is for the center of the CCD, not the center of the field of view. My proposal matches my expectation that most observatories will provide the data for the center of the field of view, and that most users will be content with that, but some algorithms, such as DCR correction, may require those values at the center of the CCD or even at different points across the CCD. In any case we have to be careful how {{IsrTask}} loads or computes this data, since one can imagine that an observatory might provide airmass, etc. at the center of each CCD instead of at the center of the field. |
| Description |
- Add {{set/getBBox}} methods for the bounding box of the exposure, an {{afw::geom::Box2I}} (not a shared pointer, thus matching exposure.getBBox()). This will be automatically updated when one retrieves a subexposure or calls exposure.setXY0 but will not be safe against changing the xy0 of the underlying masked image (just as one can, but should not, manually change the xy0 of the variance plane or mask in a masked image). - Add {{set/getField}} methods for data describing the center of the field of view; a {{shared_ptr<Field>}}, where {{Field}} is a new struct with attributes: - {{exposureId}}: exposure ID, a {{long int}}; this needs a demangler to turn it into a visit ID - {{sky}}: coordinates of the center of the field of view; an {{afw::coord::IcrsCoord}} - {{observed}}: refracted, topocentric Az/Alt; an {{afw::coord::TopocentricCoord}} - {{airmass}}, a {{double}} - {{midDate}}, a {{daf::base::DateTime}} - {{last}} local apparent sidereal time, an {{afw::geom::Angle}} - {{positionAngle}}, angle from celestial north to x axis of CCD, an {{afw::geom::Angle}} - {{airTemperature}} in C, a {{double}} - {{airPressure}} in Pascals, a {{double}} - {{humidity}} as a fraction, a {{double}} - {{observatory}} longitude, latitude and elevation of telescope; a {{afw::coord::Observatory}} - note that exposure time will remain in {{Calibrate}}, but {{Field}} documentation will say so - Add {{getAirmass(pixPos)}} and {{getObserved(pixPos)}} functions that use {{Field}}, {{Wcs}} and {{Detector}}; {{pixPos}} will default to the center of the detector. One simplified alternative is to store the airmass, position angle and observed position for the center of the CCD, rather than the center of the field, and ditch the functions that compute these values at other points on the CCD. If we do that, the name of that object will need some thought, since it must be made clear that the data is for the center of the CCD, not the center of the field of view. My proposal matches my expectation that most observatories will provide the data for the center of the field of view, and that most users will be content with that, but some algorithms, such as DCR correction, may require those values at the center of the CCD or even at different points across the CCD. In any case we have to be careful how {{IsrTask}} loads or computes this data, since one can imagine that an observatory might provide airmass, etc. at the center of each CCD instead of at the center of the field. |
- Add {{set/getBBox}} methods for the bounding box of the exposure, an {{afw::geom::Box2I}} (not a shared pointer, thus matching exposure.getBBox()). This will be automatically updated when one retrieves a subexposure or calls exposure.setXY0 but will not be safe against changing the xy0 of the underlying masked image (just as one can, but should not, manually change the xy0 of the variance plane or mask in a masked image). - Add {{set/getField}} methods for data describing the center of the field of view; a {{shared_ptr<Field>}}, where {{Field}} is a new struct with attributes: - {{exposureId}}: exposure ID, a {{long int}}; this needs a demangler to turn it into a visit ID (see DM-6912) - {{sky}}: coordinates of the center of the field of view; an {{afw::coord::IcrsCoord}} - {{observed}}: refracted, topocentric Az/Alt; an {{afw::coord::TopocentricCoord}} - {{airmass}}, a {{double}} - {{midDate}}, a {{daf::base::DateTime}} - {{last}} local apparent sidereal time, an {{afw::geom::Angle}} - {{positionAngle}}, angle from celestial north to x axis of CCD, an {{afw::geom::Angle}} - {{airTemperature}} in C, a {{double}} - {{airPressure}} in Pascals, a {{double}} - {{humidity}} as a fraction, a {{double}} - {{observatory}} longitude, latitude and elevation of telescope; a {{afw::coord::Observatory}} - note that exposure time will remain in {{Calibrate}}, but {{Field}} documentation will say so - Add {{getAirmass(pixPos)}} and {{getObserved(pixPos)}} functions that use {{Field}}, {{Wcs}} and {{Detector}}; {{pixPos}} will default to the center of the detector. One simplified alternative is to store the airmass, position angle and observed position for the center of the CCD, rather than the center of the field, and ditch the functions that compute these values at other points on the CCD. If we do that, the name of that object will need some thought, since it must be made clear that the data is for the center of the CCD, not the center of the field of view. My proposal matches my expectation that most observatories will provide the data for the center of the field of view, and that most users will be content with that, but some algorithms, such as DCR correction, may require those values at the center of the CCD or even at different points across the CCD. In any case we have to be careful how {{IsrTask}} loads or computes this data, since one can imagine that an observatory might provide airmass, etc. at the center of each CCD instead of at the center of the field. |
Oh, I agree with Jim Bosch's suggestion about VisitInfo instead of Field, and about sky and observed needing better names.
To answer Jim Bosch's comments:
- I agree that exposure time is awkward. I prefer keeping exposure time in Calib because Calib supports addition and subtraction, which automatically give the new correct exposure time. But I also agree that it more naturally belongs in Field. It might suffice to add getExposureTime to ExposureInfo itself, in which case it matters less where that value lives.
- I agree that Field is not a great name. VisitInfo sounds reasonable, though it obscures the fact that the information is for the boresight. It will presumably be empty for coadds.
- For positionAngle how about focalPlaneOrientation. Also we may want getDetectorOrientation(pixpos), though I would rather not bother until we have a need.
- for last we use localApparentSiderealTime or break the case rules and use LAST. I slightly prefer the former, verbose option. It's not like it'll be needed very often.
| Description |
- Add {{set/getBBox}} methods for the bounding box of the exposure, an {{afw::geom::Box2I}} (not a shared pointer, thus matching exposure.getBBox()). This will be automatically updated when one retrieves a subexposure or calls exposure.setXY0 but will not be safe against changing the xy0 of the underlying masked image (just as one can, but should not, manually change the xy0 of the variance plane or mask in a masked image). - Add {{set/getField}} methods for data describing the center of the field of view; a {{shared_ptr<Field>}}, where {{Field}} is a new struct with attributes: - {{exposureId}}: exposure ID, a {{long int}}; this needs a demangler to turn it into a visit ID (see DM-6912) - {{sky}}: coordinates of the center of the field of view; an {{afw::coord::IcrsCoord}} - {{observed}}: refracted, topocentric Az/Alt; an {{afw::coord::TopocentricCoord}} - {{airmass}}, a {{double}} - {{midDate}}, a {{daf::base::DateTime}} - {{last}} local apparent sidereal time, an {{afw::geom::Angle}} - {{positionAngle}}, angle from celestial north to x axis of CCD, an {{afw::geom::Angle}} - {{airTemperature}} in C, a {{double}} - {{airPressure}} in Pascals, a {{double}} - {{humidity}} as a fraction, a {{double}} - {{observatory}} longitude, latitude and elevation of telescope; a {{afw::coord::Observatory}} - note that exposure time will remain in {{Calibrate}}, but {{Field}} documentation will say so - Add {{getAirmass(pixPos)}} and {{getObserved(pixPos)}} functions that use {{Field}}, {{Wcs}} and {{Detector}}; {{pixPos}} will default to the center of the detector. One simplified alternative is to store the airmass, position angle and observed position for the center of the CCD, rather than the center of the field, and ditch the functions that compute these values at other points on the CCD. If we do that, the name of that object will need some thought, since it must be made clear that the data is for the center of the CCD, not the center of the field of view. My proposal matches my expectation that most observatories will provide the data for the center of the field of view, and that most users will be content with that, but some algorithms, such as DCR correction, may require those values at the center of the CCD or even at different points across the CCD. In any case we have to be careful how {{IsrTask}} loads or computes this data, since one can imagine that an observatory might provide airmass, etc. at the center of each CCD instead of at the center of the field. |
- Add {{set/getBBox}} methods for the bounding box of the exposure, an {{afw::geom::Box2I}} (not a shared pointer, thus matching exposure.getBBox()). This will be automatically updated when one retrieves a subexposure or calls exposure.setXY0 but will not be safe against changing the xy0 of the underlying masked image (just as one can, but should not, manually change the xy0 of the variance plane or mask in a masked image). - Add {{set/getField}} methods for data describing the center of the field of view; a {{shared_ptr<Field>}}, where {{Field}} is a new struct with attributes: - {{exposureId}}: exposure ID, a {{long int}}; this needs a demangler to turn it into a visit ID (see DM-6912) - {{sky}}: coordinates of the center of the field of view; an {{afw::coord::IcrsCoord}} - {{observed}}: refracted, topocentric Az/Alt; an {{afw::coord::TopocentricCoord}} - {{airmass}}, a {{double}} - {{midDate}}, a {{daf::base::DateTime}} - {{last}} local apparent sidereal time, an {{afw::geom::Angle}} - {{positionAngle}}, angle from celestial north to x axis of focal plane, an {{afw::geom::Angle}} - {{airTemperature}} in C, a {{double}} - {{airPressure}} in Pascals, a {{double}} - {{humidity}} as a fraction, a {{double}} - {{observatory}} longitude, latitude and elevation of telescope; a {{afw::coord::Observatory}} - note that exposure time will remain in {{Calibrate}}, but {{Field}} documentation will say so - Add {{getAirmass(pixPos)}} and {{getObserved(pixPos)}} functions that use {{Field}}, {{Wcs}} and {{Detector}}; {{pixPos}} will default to the center of the detector. One simplified alternative is to store the airmass, position angle and observed position for the center of the CCD, rather than the center of the field, and ditch the functions that compute these values at other points on the CCD. If we do that, the name of that object will need some thought, since it must be made clear that the data is for the center of the CCD, not the center of the field of view. My proposal matches my expectation that most observatories will provide the data for the center of the field of view, and that most users will be content with that, but some algorithms, such as DCR correction, may require those values at the center of the CCD or even at different points across the CCD. In any case we have to be careful how {{IsrTask}} loads or computes this data, since one can imagine that an observatory might provide airmass, etc. at the center of each CCD instead of at the center of the field. |
- I don't think "field" implies boresight – as Jim says the term's overloaded in astronomy. Things like pressure don't apply to the boresight, but things dependent on the telescope orientation do.
- rereading
DM-5502I don't think I praise observatory's sophistication in airmass calculations! I wanted a method with an argument asking for the full-precision version - The IAU abolished mean and apparent sidereal time (we're supposed to use ERA instead), and as LST is not used for anything very precise (that I know of) I don't think I'd be too picky here. I stand by "getLocalSiderealTime()" – and if I'm wrong about needing a precise value, add an argument "apparent=true".
- I worry that it is out of scope for ExposureInfo to include an an accurate computation of airmass. That seems like an algorithm we might want to be able to tweak or configure. I would be much happier taking that value from somewhere (the observatory or ISR). However, we may need the value at the center of the detector to properly compensate for DCR. I felt it was reasonable to include code that would determine that from the airmass at the center of the field, since I think that computation can be fairly basic and still give acceptable accuracy. If not, we should probably store the airmass at the center of the detector and not provide any means of computing it elsewhere in ExposureInfo.
- I agree that VisitInfo does not imply boresight, so I will add that to the sky and observed.
- I will use localSiderealTime and document that it is apparent. I see no point to offering mean.
- VisitInfo is a struct, so access is by field name rather than get/set methods. Furthermore, my hope is that it will have no methods.
I would like to echo Jim Bosch's concerns about naming, and suggest that it is not too late to rationalize this. I'm also concerned about making sure that the organization of the metadata does not make assumptions that certain types of metadata will always be there. We should keep in mind that our software will be handling:
- images that are not true "exposures" (e.g., bias and dark frames, camera engineering data, images taken when the Camera is on the Summit CCOB)
- exposures that are not on the sky (e.g., dome flats and CBP exposures) for which sky-related quantities are irrelevant, and where I think it would be preferable not to have to supply "dummy values" for the sky quantities (but note that quantities such as alt/az and the Camera rotation angle remain relevant)
- exposures for which a straightforward x/y <--> ra/dec transform is not applicable (images from the Calypso spectrograph)
- sky exposures that involve non-sidereal tracking (for which, at the very least, the meaning of the sky coordinates need to be precisely defined - e.g., coordinates at the midpoint of the exposure)
Can we represent these differences by a core object that is meaningful for every image, adding an additional metadata object that is meaningful for every exposure, and a further one that is meaningful for every sky exposure?
Thus, perhaps the main MaskedImage+metadata class could be a SomethingImage (surely we can think of a name), to which an ExposureMetadata object would get added for exposures, and a SkyMetadata object for on-sky exposures?
Just for my interest (possibly off-topic to the RFC): what are the interfaces that allow the retrieval of the exposure start/midpoint/stop and vignetting-vs-time profile as a function of location on the focal plane?
As far as information that may not always be there: VisitInfo can be an empty pointer if not relevant. My plan was to set it empty for coadds and be done with it. However, some of the use cases you present need a subset of the information.
We have never claimed our pipeline will handle spectroscopic data. Is that in scope? If so, it needs some careful thought. We plan to use AST for the new WCS, and it can easily handle spectroscopic data, but functions such as getObserved and getAirmass would almost certainly be useless or too complicated to bother. Even our current camera geometry object does not seem relevant to a spectrograph. If we really are attempting that level of generality then I think we have a lot of rework ahead of us.
For non-sidereal tracking I think the most we can expect is that the time at which the data is valid is specified (and I vote for the middle of the exposure).
If you want to clean up the Exposure/ExposureInfo naming I suggest a new RFC. At that point I'd also suggest we expand ExposureInfo just far enough to support jointcal for now, using this RFC, then switch over to the new system as somebody has time to design it.
The analysis of the data from the Calypso spectrograph certainly needs to be done in the context of the DM stack, and I think that is documented as being in-scope. How much of the code can be reused is a matter for the calibration developers...
We need to handle calypso data, but I think that that's orthogonal to this discussion. No decision to expose AST to our code has been taken; what was decided was that we'd use parts of it to support our need for mappings between pixels and sky coordinates.
Re class/struct: Why is this a struct rather than a class? As i said above, I think that some of things that we need to offer are derived from others, so using methods allows us to normalise the data.
Re airmass: The initial computation can be the simple one, but putting it behind a method allows us to be more sophisticated if we need to be.
I think Gregory Dubois-Felsmann raises a good point about different kinds of exposures. We probably don't need to deal with all of it in this RFC, but it's worth at least taking a look.
Here is a summary of my understanding of the types of images we may deal with and their associated metadata. Note that coadds are only discussed briefly at the end.
Types of image:
- science image: telescope tracks the sky
- calibration image, e.g. dome flat, bias or sky flat; telescope fixed in az/alt
- spectrum
- calibration spectrum (much like a calibration image, so not called out below)
Metadata:
- observatory longitude and latitude
- visit info:
- boresight sky: varies for calibrationimage, but could be nan in that case, since the RA/Dec is not of interest
- boresight observed (refracted topocentric) az/alt: varies for all but calibration data
- orientation of sky w.r.t. focal plane: varies for calibration image, may vary for spectrum
- orientation of horizon w.r.t. focal plane: varies for science image, may vary for spectrum
- weather information
- date information at exposure midpoint:
- TAI date
- UT1 date
- LAST (or ERA if Robert prefers; I'd appreciate a reference to that as I've not heard of it)
- exposure time
- photometric zero point and uncertainty: I confess I don't know if this means anything for cals or spectra
- detector: much of the data is useful for all types of images (e.g. the amp info table), but the current positional transformations don't make sense for a spectrum; doing anything about that is outside the scope of this RFC
- WCS: standard sky<->pixels for science image, something else for a spectrum, may be absent for calibration image. Doing anything about WCS for spectra is outside the scope of this RFC, but AST offers an obvious solution
- visit info translated to CCD position:
- sky position and orientation: users obtain from WCS for science images, so no special support needed
- observed az/alt and orientation of horizon: can compute from visit info and camera geometry for science images and cals, but it's not trivial so we should offer a way to compute these
I propose that metadata that varies over the exposure will be specified at the midpoint of the exposure. I'd like to have a single standard, for predictability.
Coadds:
We can coadd any of the types of data above. Common to all types:
- LSST has a nice object to keep track of what images go into a coadd
- No visit info
- coadded science images:
- WCS is normal
- no detector
- no exposure time, since it varies across the image (and not smoothly)
- no photometric zero point and uncertainty, since it varies across the image (and not smoothly)
- no observed az/alt and orientation of horizon
- coadded calibration data:
- sky position is not relevant
- Az/alt may be the same for all exposures in a coadd, but that is not guaranteed; I hope we can omit it
- exposure time is normal
- detector is normal
- coadded spectra: I am not familiar enough with spectral data to say
Conclusion:
- VisitInfo seems like a nice object to have; a few bits of it will not be defined for some kinds of exposures.
- Exposure time may need to live on its own. If it goes in VisitInfo then we need an alternate location for coadded cals, or a largely blank VisitInfo.
- What to do about az/alt for coadded cals? At least in some cases this will be the same for all exposures that go into the coadd, but do we really care enough to provide it in that case?
Russell Owen the Earth rotation angle (ERA) is defined in this document: http://aa.usno.navy.mil/publications/docs/Circular_179.pdf.
What to do about az/alt for coadded cals? At least in some cases this will be the same for all exposures that go into the coadd, but do we really care enough to provide it in that case?
I'm not answering your question directly... but...
The alt/az is probably not very interesting for dome flats, once an optimal pointing has been determined during commissioning (it may well be the naive pointing that you would get just from the screen geometry, but I am assuming we'll explore around that point a bit to explore uniformity, etc.).
But for CBP images, the alt/az is not fixed and is required to be known in order to understand the entry angle of the CBP beam to the telescope optics. Both the telescope and the CBP must be steered in order to map out the full space of the telescope aperture and field of view.
(Note that this means that the CBP instrumental metadata also becomes important - but I am not suggesting that be merged into the objects proposed above. I propose that we invent a separate CBP metadata object and let the appropriate instance be retrieved from the Butler given an image ID.)
So, if coaddition of CBP images is needed* it would be important to be able to know the alt/az of the stack (if it was constant across the coadd).
*: One case where I'm pretty confident we'll need coaddition of CBP frames is in the measurement of crosstalk, in order to be able to measure crosstalk reliably below the noise floor. Unfortunately this is a weak example of one where alt/az really matters, because beam angle is not of much interest in a crosstalk measurement. Maybe Robert Lupton or Merlin Fisher-Levine can supply a better example...
I admit to my taste in these matters being very C++-flavored, but regarding Robert Lupton's:
Why is this a struct rather than a class? As i said above, I think that some of things that we need to offer are derived from others, so using methods allows us to normalise the data.
I can't count the number of times that this turned out to be useful in expected and unexpected ways in the BaBar data model.
Although ensuring that we don't obviously break the calibration data with this interface is good (and from Russell Owen's comment above, I think we're fine), designing a new calibration metadata interface is a bit out of scope for this. I need a better metadata interface VerySoonNow™ for jointcal, which is what prompted this whole thing.
I do agree that VisitInfo (née Field) should be a class, not a struct, for all the reasons stated above.
I don't especially prefer ERA, but it is the Official IAU (and thus erfa/sofa) quantity.
Another try at the proposal based on the feedback so far. Changes include:
- rename Field to VisitInfo
- moved exposure time from Calib to VisitInfo
- adding "boresight" some names
- adding a field for UT1
- change local apparent sidereal time to earth rotation angle; we may as well meet the future
- reorder fields to keep related data together
- redo orientation, including adding a field for rotation type
- moving weather information into a new class
- eliminate computations of various values at the center of the detector; I suspect a different mechanism than instance methods may be preferred, especially given the number of different kinds of images this metadata must support. I suggest leaving that for another RFC.
Updated Proposal:
- Add set/getBBox methods to ExposureInfo for the bounding box of the exposure, an afw::geom::Box2I (not a shared pointer, thus matching exposure.getBBox()). This will be automatically updated when one retrieves a subexposure or calls exposure.setXY0 but will not be safe against changing the xy0 of the underlying masked image (just as one can, but should not, manually change the xy0 of the variance plane or mask in a masked image).
- Add set/getVisitInfo methods to ExposureInfo for data describing the center of the field of view; a shared_ptr<VisitInfo>, where VisitInfo is a new class that offers direct struct-like access (rather than get/set methods) to the following fields:
- observatory longitude, latitude and elevation of telescope; an afw::coord::Observatory
- exposureId: exposure ID, a long int; this needs a demangler to turn it into a visit ID (see DM-6912)
- tai TAI date of observation, a daf::base::DateTime
- ut1 UT1 date of observation, a daf::base::DateTime
- era earth rotation angle, an afw::geom::Angle
- exposureTime, a double
- boresightSky: ICRS position of the boresight; an afw::coord::IcrsCoord
- observedAzAlt: refracted, apparent topocentric position of the boresight; an afw::coord::TopocentricCoord, which is not quite right, but the closest class we have
- boresightRotAngle, angle of coordinate system specified by boresightRotType with respect to detector, a double*
- boresightRotType, rotation type lsst::afw::coord::RotationType enum; this indicates how the rotator was being controlled and defines the coordinate system of boresightRotAngle; detector X axis is flipped, if necessary, to match the handedness of the other coordinate system:
- SKY: orientation of sky: at 0 north is along detector Y and east is along detector X; at 90 east is along detector Y and north is along detector -X
- HORIZON: orientation of az/alt: at 0 alt is along detector Y and az is along detector X; at 90 az is along detector Y and alt is along detector -X
- MOUNT: rotator set to a specific mount (actuator) angle; the exact meaning of boresightRotAngle will be specific to the telescope and instrument port
- NONE: no rotator or rotation angle unknown (boresightRotType will also be nan)
- boresightAirmass, a double
- weather, an instance of lsst::afw::coord::Weather a class containing these fields (more may be added later) which are accessed as struct fields:
- airTemperature in C, a double
- airPressure in Pascals, a double
- humidity as a fraction, a double
Notes:
- Varying values are correct at the midpoint of the exposure.
- Unknown values are nan.
- VisitInfo must not be a null pointer; ExposureInfo will enforce that
- Although some of the above values can be computed from others, none of these values are computed on the fly. This is in order to better support incomplete information in raw files; for instance earth rotation angle may be supplied, but not UT1, or airmass supplied but not weather data. LSST should provide code that performs these computations, but doing so is outside the scope of this RFC.
- I have seen a lot of push-back on using a struct, but am unclear if this means folks think all fields should be accessed using get/set methods. If so, I'm willing to make that change. I agree that VisitInfo may grow to have useful methods.
VisitInfo must not be a null pointer; ExposureInfo will enforce that
I'd have expected it to be null for coadd images; certainly very few of these values are appropriate for coadds; perhaps this suggests that those that are (BBox, exposureId) ought to be refactored out of it?
Jim Bosch get/setBBox is proposed as a method on ExposureInfo. You make a good point about VisitInfo for coadds. I agree it can be null, but I suggest that it should never be null for a science image, and algorithms are free to error out if it is.
I agree it can be null, but I suggest that it should never be null for a science image, and algorithms are free to error out if it is.
+1
Thanks for the clarification on get/setBBox; I agree that having it on ExposureInfo makes sense.
This is small, but if I was looking through this list to get the field center in Ra/Dec, I would never guess that boresightSky was it. I would add something to make it clear that it was a coordinate; boresightRaDec or boresightSkyCoord maybe.
Colin Slater
added a comment - This is small, but if I was looking through this list to get the field center in Ra/Dec, I would never guess that boresightSky was it. I would add something to make it clear that it was a coordinate; boresightRaDec or boresightSkyCoord maybe. Colin Slater I see your point about boresightSky but will push back gently: sky is a term we already use in our existing WCS classes, e.g. pixelToSky. Still, I'm willing to go with pixelToSkyCoord if you feel strongly about it. Clarity may be worth the extra typing.
The difference is the context; I know members of Wcs are going to deal with coordinates. ExposureInfo is a grab bag of dates, times, temperatures, angles, etc., so adding Coord can be a big help to give that context. Otherwise my first guess for what "sky" means would be sky brightness.
Colin Slater
added a comment - The difference is the context; I know members of Wcs are going to deal with coordinates. ExposureInfo is a grab bag of dates, times, temperatures, angles, etc., so adding Coord can be a big help to give that context. Otherwise my first guess for what "sky" means would be sky brightness. After some consideration and discussion, I think all of the above listed values should be computed and written as part of ISR, and thus be fields (not getters/setters). So, I don't care so much about this being a Struct (modulo wanting to add other methods later).
In the interest of not having to re-write the various calculations for every obs_* I think we'll want some helper methods in the obs_* parent class.
I believe I'm happy with the above, with Colin's "Coord" addition (actually, I think I'd prefer "RaDec" to "Coord", but don't care that much, so long as one of them goes in the name).
I don't think we can assume that ISR can write all these – what if there's no astrometric information provided in the headers?
I thought that the code standards mandated getters/setters in C++. This is very java and I'd be willing to change it, but unless that's happened while I was asleep, this isn't the place to make that change.
Robert Lupton I agree that we can't assume ISR can write all these. I think the best we can do is the following, but if you have something else in mind, please explain:
- If an item is provided, use it, with whatever translation is required.
- If an item is not directly provided, but can be computed from other data, compute it.
- If an item cannot be determined, set it to nan.
I agree we will use setters and getters in the C++ code for the reasons Robert Lupton stated. With that change, I think the RFC is ready to be adopted.
Adopted as follows:
- Add set/getBBox methods to ExposureInfo for the bounding box of the exposure, an afw::geom::Box2I (not a shared pointer, thus matching exposure.getBBox()). This will be automatically updated when one retrieves a subexposure or calls exposure.setXY0 but will not be safe against changing the xy0 of the underlying masked image (just as one can, but should not, manually change the xy0 of the variance plane or mask in a masked image).
- Add set/getVisitInfo methods to ExposureInfo for data describing the center of the field of view; a shared_ptr<afw::image::VisitInfo>, where afw::image::VisitInfo is a new class that offers getters and setters for the following fields:
- exposureId: exposure ID, a long int; this needs a demangler to turn it into a visit ID (see DM-6912)
- exposureTime, a double
- date date of observation, a daf::base::DateTime
- ut1 UT1 MJD date of observation, a double; I had hoped to use daf::base::DateTime, but it is intended for uniform time systems (or semi-uniform in the case of UTC) and cannot easily be made to support UT1
- era earth rotation angle, an afw::geom::Angle
- boresightRaDec: ICRS position of the boresight; an afw::coord::IcrsCoord
- observedAzAlt: refracted, apparent topocentric position of the boresight; an afw::coord::Coord. Note: I chose not to use afw::coord::TopocentricCoord because it's not quite right and because it contains an afw::coord::Observatory and I would rather store that separately.
- boresightAirmass, a double
- boresightRotAngle, angle of coordinate system specified by boresightRotType with respect to detector, an afw::geom::Angle
- boresightRotType, rotation type, an afw::coord::RotationType enum; this indicates how the rotator was being controlled and defines the coordinate system of boresightRotAngle; detector X axis is flipped, if necessary, to match the handedness of the other coordinate system:
- SKY: orientation of sky: at 0° north is along detector Y and east is along detector X; at 90° east is along detector Y and north is along detector -X
- HORIZON: orientation of az/alt: at 0° alt is along detector Y and az is along detector X; at 90° az is along detector Y and alt is along detector -X
- MOUNT: rotator set to a specific mount (actuator) angle; the exact meaning of boresightRotAngle will be specific to the telescope and instrument port
- NONE: no rotator or rotation angle unknown (boresightRotType will also be nan)
- observatory longitude, latitude and elevation of telescope; an afw::coord::Observatory
- weather, an afw::coord::Weather an immutable class containing these fields (more may be added later) which are accessed as with getters:
- airTemperature in C, a double
- airPressure in Pascals, a double
- humidity as a fraction, a double
Notes:
- Varying values are correct at the midpoint of the exposure.
- Unknown values are nan.
- The CameraMapper std_raw method is responsible for setting what values it can, based on existing data from the raw header, and setting the remaining values to nan.
- In the long run we will also want to be able to compute some items at the center of the detector, and perhaps other points on the detector. Such items include date of exposure, airmass and az/alt. That is out of scope of this RFC. However, possible solutions include adding a new object to VisitInfo and beefing up the WS to add additional frames (which is likely a fine way to deal with az/alt).
- If the telescope is tracking in an unusual coordinate system or is tracking a moving object, then this is not a complete model. However, it is simpler than a more general model and probably sufficient.
| Resolution | Done [ 10000 ] | |
| Status | Proposed [ 10805 ] | Adopted [ 10806 ] |
| Remote Link | This issue links to "Page (Confluence)" [ 14358 ] |
| Remote Link | This issue links to "Page (Confluence)" [ 14358 ] | This issue links to "Page (Confluence)" [ 14358 ] |
Krzysztof Findeisen
made changes -
| Remote Link | This issue links to "Page (Confluence)" [ 14358 ] | This issue links to "Page (Confluence)" [ 14358 ] |
| Remote Link | This issue links to "Page (Confluence)" [ 14358 ] | This issue links to "Page (Confluence)" [ 14358 ] |
| Remote Link | This issue links to "Page (Confluence)" [ 14358 ] | This issue links to "Page (Confluence)" [ 14358 ] |
| Remote Link | This issue links to "Page (Confluence)" [ 14358 ] | This issue links to "Page (Confluence)" [ 14358 ] |
| Remote Link | This issue links to "Page (Confluence)" [ 14358 ] | This issue links to "Page (Confluence)" [ 14358 ] |
| Remote Link | This issue links to "Page (Confluence)" [ 14358 ] | This issue links to "Page (Confluence)" [ 14358 ] |
| Remote Link | This issue links to "Page (Confluence)" [ 14358 ] | This issue links to "Page (Confluence)" [ 14358 ] |
| Remote Link | This issue links to "Page (Confluence)" [ 14358 ] | This issue links to "Page (Confluence)" [ 14358 ] |
Though I'm baised as I had a hand in constructing this, I like the proposal as given. It would be useful to know more about the airmass values that CFHT/HSC provide with their files: do those calculations incorporate more site knowledge than one could get from just the basic weather conditions?