Details
-
Type:
Bug
-
Status: Invalid
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: afw
-
Labels:
Description
exposureId 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. exposureId should be moved back into ExposureInfo.
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']). We could have makeRawVisitInfo() add the butler visitId to the new visitInfo.visitId that this ticket will add.
Marking this Critical because we should fix this ASAP, before use of visitInfo.exposureId becomes widespread. This is an API-breaking change, so it may need an RFC?
See this discussion that generated this ticket:
https://lsstc.slack.com/archives/C3UCAEW3D/p1517724549000034
Attachments
Issue Links
Activity
| Field | Original Value | New Value |
|---|---|---|
| Description |
[{exposureId}} 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. {{exposureId}} should be moved back into {{ExposureInfo}}.
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']}}). We could have {{makeRawVisitInfo()}} add the butler visitId to the new {{visitInfo.visitId}} that this ticket will add. Marking this Critical because we should fix this ASAP, before use of {{visitInfo.exposureId}} becomes widespread. This is an API-breaking change, so it may need an RFC? See this discussion that generated this ticket: https://lsstc.slack.com/archives/C3UCAEW3D/p1517724549000034 |
{{exposureId}} 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. {{exposureId}} should be moved back into {{ExposureInfo}}.
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']}}). We could have {{makeRawVisitInfo()}} add the butler visitId to the new {{visitInfo.visitId}} that this ticket will add. Marking this Critical because we should fix this ASAP, before use of {{visitInfo.exposureId}} becomes widespread. This is an API-breaking change, so it may need an RFC? See this discussion that generated this ticket: https://lsstc.slack.com/archives/C3UCAEW3D/p1517724549000034 |
| Comment | [ I think we need a field like this in VisitInfo, though I agree that it is not being set correctly at this time. I suggest we retain it, but change our code to not set it until we can set it properly. Furthermore, we can probably modify most obs packages to set it correctly without much difficulty. A standardized, non-obs-specific means of setting it will have to wait, but meanwhile we can have a proper visit ID. ] |
| Resolution | Done [ 10000 ] | |
| Status | To Do [ 10001 ] | Invalid [ 11005 ] |
Krzysztof Findeisen
made changes -