-
Notifications
You must be signed in to change notification settings - Fork 3
DM-53634: Add daytime AP pipeline and scripts #250
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.
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
Conversation
Prompt Processing does not save `dia_source_unstandardized` so switch to an output catalog that will be available to the afterburner
7aed524 to
7de4ad0
Compare
|
It could be useful to enable dumping of APDB metrics to the log. It needs one envvar set in your job: |
11d6216 to
dbbcd98
Compare
| AP daytime pipeline to process images that were missed by Prompt Processing and generate additional plots and metrics. | ||
| instrument: lsst.obs.lsst.LsstCam | ||
| imports: | ||
| - location: $AP_PIPE_DIR/pipelines/_ingredients/ApPipeWithIsrTaskLSST.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be $AP_PIPE_DIR/pipelines/LSSTCam/ApPipe.yaml instead? I assume the filterDiaSourcePostReliability config needs to match.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even better, I can move the release_id config setting to the bps yaml, and remove ApPipe-daytime entirely.
| imports: | ||
| - location: $AP_PIPE_DIR/pipelines/_ingredients/ApPipeWithIsrTaskLSST.yaml | ||
| exclude: | ||
| - getRegionTimeFromVisit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not sure why this exclusion is needed. getRegionTimeFromVisit was specifically intended for batch processing (since it uses the visitinfo, it's more precise and more accurate than PP's estimate from the nextVisit message).
Unfortunately, it turns out that ap_verify removed it when preprocessing was removed on lsst/ap_verify#238, so a bug may have creeped in without our noticing. 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am putting getRegionTimeFromVisit back in the daytime pipeline, and will work around the visit-ordering issues.
e41f016 to
9cc5752
Compare
erinleighh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good to me! I just have a small management question that I'll put on the Jira ticket.
9cc5752 to
df67dd1
Compare
No description provided.