-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add comcam full default pipeline #252
Conversation
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.
Let's change this to something like comCamRapidAnalysisPipeline.yaml
config: | ||
donutSelector.useCustomMagLimit: True | ||
donutSelector.sourceLimit: 5 | ||
cutOutDonutsScienceSensorTask: |
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.
Let's use the group pairer. See https://github.com/lsst-ts/ts_wep/blob/develop/tests/testData/pipelineConfigs/testCutoutsFamPipelineGroupPairer.yaml#L6 for an example.
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.
Out of curiosity, what does this do, exactly?
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.
It controls how exposures are paired up when you run in batch mode (this "pairer" looks at the value of the "group" dimension to find pairs). It's also required right now if you want to append new aos pipeline results to an existing output collection.
isr: | ||
class: lsst.ip.isr.isrTask.IsrTask | ||
config: | ||
connections.outputExposure: "postISRCCD" | ||
doBias: False | ||
doVariance: False | ||
doLinearize: False | ||
doCrosstalk: False | ||
doDefect: False | ||
doNanMasking: False | ||
doInterpolate: False | ||
doBrighterFatter: False | ||
doDark: False | ||
doFlat: False | ||
doApplyGains: True | ||
doFringe: False | ||
doOverscan: True | ||
python: OverscanCorrectionTask.ConfigClass.fitType = 'MEDIAN' |
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.
@mfisherlevine , how do these ISR settings compare to the quicklook pipeline?
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.
Errr, very different indeed, to be honest! quicklook is basically the same as DRP expect with doBrighterFatter
set to False
because it's slow, but everything else is super quick and also necessary for pretty much all processing. So the good news is that you'd be getting all of that for free, the bad news is that it's nothing like your current setup...
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.
Okay, I vote we adopt the quicklook settings. Guillem, I think you can copy those from https://github.com/lsst/drp_pipe/blob/main/pipelines/_ingredients/DRP-full.yaml#L12 and https://github.com/lsst/drp_pipe/blob/main/pipelines/LSSTComCam/quickLook.yaml#L8.
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 think you should be able to import them actually, rather than copying them. But then maybe this would need to live in that package, not sure. I'd have no problem with that being the case though, personally.
description: | | ||
AOS Donut visualization plotting tasks. This step generates plots | ||
(including the pyramid residual and donut gallery) and | ||
tables for the AOS visit. |
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.
Add blank line at end please.
@@ -0,0 +1,75 @@ | |||
description: wep basic processing test pipeline |
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.
Can we change the directory name to something more descriptive like "production"?
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.
Great suggestion, i really didn't like fullPipelines but ran out of naming imagination
LGTM. Thanks Guillem! |
639e6bb
to
7f8e6ac
Compare
description: rapid analysis pipeline for ComCam | ||
instrument: lsst.obs.lsst.LsstComCam | ||
tasks: | ||
generateDonutDirectDetectTask: |
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.
Sorry, I know this has been hanging around a while but while preparing our notebooks for analysis I've been thinking aboutgenerateDonutFromRefitWcsTask
as our default rather than generateDonutDirectDetectTask
. What do you think? fromRefitWcs
might not work if the pointing model is really bad at the start but I think we should talk about trying to use it. Anyway, maybe something to bring up during an AOS meeting.
7f8e6ac
to
b861627
Compare
No description provided.