DM-27929: refactor meas_extensions_scarlet and skip sky objects #38
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
March 22, 2021 09:27
While implementing a fix to skip sky objects I noticed that there are a number of places where columns were not being appropriately updated. So this commit refactors ScarletDeblendTask to make it less likely that similar errors can occur in the future, and also adds tests to check that columns are being persisted properly. The major change is that now all of the source updates are occurring on the input (mergeDet) catalog, and only at the end are the single band catalogs created and updated with band specific information. An added benefit is that deblended sources have the same source ID across catalogs, and are also now continuous catalogs.
This task was missing tests to ensure that the appropriate output columns were being generated, and that their values are correct. So this commit adds some much needed testing.
taranu
reviewed
Mar 25, 2021
| # Set the psf key based on whether or not the source was | ||
| # deblended using the PointSource model. | ||
| # This key is not that useful anymore since we now keep track of | ||
| # `modelType`, but we continue to propagate it in case code downstream | ||
| # is expecting it. | ||
| src.set(self.psfKey, scarletSource.__class__.__name__ == "PointSource") | ||
| src.set(self.modelTypeKey, scarletSource.__class__.__name__) | ||
| # We set the runtime to zero so that summing up the |
There was a problem hiding this comment.
Set the runtime to zero as opposed to what? If you're zeroing out some overhead time, is there a way to add that up separately?
fred3m
force-pushed
the
tickets/DM-27929
branch
from
d2ad248
to
b850dbc
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While implementing the code to skip sky sources, the goal of DM-27929, I discovered that inconsistencies in the way that primary sources are handled lead to columns in primary sources not being updated correctly. It has also been quite a while since meas_extensions_scarlet was first created and it needed a face-lift anyway, so this PR refactors meas_extensions_scarlet to do all of the deblending in the input (mergedDet) catalog to make sure that all of the sources have their flags set appropriately, then the band-dependent columns and footprints are set in individual catalogs that are copied from the original.