Skip to content
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

DM-36885: Ensure cp_pipe only uses a different ISR output when needed #159

Merged
merged 1 commit into from Nov 8, 2022

Conversation

czwa
Copy link
Contributor

@czwa czwa commented Nov 4, 2022

Switch cpSky to operate on postISRCCD datasetType.

@czwa czwa requested a review from leeskelvin November 4, 2022 21:22
@@ -6,7 +6,6 @@ tasks:
isr:
class: lsst.ip.isr.isrTask.IsrTask
config:
connections.outputExposure: cpSkyIsr
overscan.fitType: 'MEDIAN_PER_ROW'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can tell, the ISR task for Latiss in cpSky is configured differently to the main Latiss ISR task. When I compare the configs for this cpSky ISR task on this ticket against the default Latiss ISR task, this is my diff:

< config.overscan.fitType='MEDIAN_PER_ROW'
---
> config.overscan.fitType='MEDIAN'
204c204
< config.doCrosstalk=True
---
> config.doCrosstalk=False

Do these ISR task over-rides need to be upstreamed to the main Latiss ISR config over-rides, or can they be removed here?

Copy link
Contributor

@leeskelvin leeskelvin Nov 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference, Latiss ISR task configs generated using:

pipetask build -t lsst.ip.isr.isrTask.IsrTask --instrument lsst.obs.lsst.Latiss --show config

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doCrosstalk=True has been an optimistic hope for a while. I continue to be optimistic that we'll have a calibration soon (there are Camera Team produced ones that are probably sufficient for now).
We probably should switch the overscan method. I'll check for concerns next week.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having chatted on Slack about this, I think these config differences aren't too problematic. They can be checked after the fact too, so perhaps this is a non-issue. If you're happy with this Chris, then I'm happy too!

@czwa czwa merged commit ff196a9 into main Nov 8, 2022
@czwa czwa deleted the tickets/DM-36885 branch November 8, 2022 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants