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-33822 Fix WCS bug in DCR templates #208

Merged
merged 5 commits into from Mar 1, 2022
Merged

DM-33822 Fix WCS bug in DCR templates #208

merged 5 commits into from Mar 1, 2022

Conversation

isullivan
Copy link
Contributor

No description provided.

Copy link
Contributor

@kherner kherner left a comment

Choose a reason for hiding this comment

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

Everything looks OK except for the one comment in dcrModel.py

if amplifyModel > 1:
if refModel is None:
refModel = self.getReferenceImage(bbox)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not quite following the logic here. Before one would run self.getReferenceImage(bbox) once before the for loop. Now there is code to run it only the first time through the loop. What's the advantage? The only one I see is in case the first time through has a None subfilter, you would skip it. Am I missing something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The advantage is that getReferenceImage() does not have to be run during image differencing. amplifyModel is an option that is only used while building the DcrCoadds. I'll add a comment to make it more clear.

@isullivan isullivan merged commit a1767bb into main Mar 1, 2022
@isullivan isullivan deleted the tickets/DM-33822 branch March 1, 2022 03:17
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