DM-34826: Remove unnecessary characterize plugins from ap pipline #110
Conversation
There was a problem hiding this comment.
The changes look good, though I noted a few other plugins that maybe we don't need.
In the future, please try to break up such changes into individual commits (e.g., one for gaap, one for hsm, etc.). Seeing the plugins all at once made it much harder to check that all related configs had really been removed.
| @@ -73,8 +73,6 @@ | |||
| config.detection.isotropicGrow = True | |||
There was a problem hiding this comment.
Do we want to keep the base_ClassificationExtendedness plugin on line 54?
There was a problem hiding this comment.
As with DECam above, this isn't adding the plugin, but rather modifying its config. I'll do some further digging on the sfm.py default plugins. Those might be a bit harder to remove; see sfm.py:139 we'd have to at minimum remove items from that config list, instead of just removing lines that add new plugins as was done on this ticket.
HSM is preferred over SdssShape, but we only need SourceMoments and PsfMoments for AP.
|
@kfindeisen : I've pushed an update that re-adds part of the the shapeHSM plugin. I've updated the jira ticket: this gets rid of all metric differences before association, and only three different association metrics (see the ticket). Please take a look at the new commit. |
|
|
||
| config.plugins.names |= ["ext_shapeHSM_HsmSourceMoments", "ext_shapeHSM_HsmPsfMoments"] | ||
| config.slots.shape = "ext_shapeHSM_HsmSourceMoments" | ||
| config.slots.psfShape = "ext_shapeHSM_HsmPsfMoments" |
There was a problem hiding this comment.
I assume you meant for this file to be shared?
There was a problem hiding this comment.
Yes, it's intentionally shared between them. Eventually I think this config will end up in {{characterizeImageConfig.setDefaults}} via DM-34969, but this will do for us for now.
No description provided.