DM-15635: Approximate filter throughput for DcrCoadds #227
Conversation
| modelWidth = pexConfig.Field( | ||
| dtype=int, | ||
| doc="Width of the region around detected sources to include in the DcrModel." | ||
| "Set to a negative number to disable, which will include all pixels.", |
There was a problem hiding this comment.
Unless you care to convince me otherwise, I'd prefer an error to be raised if a nonsensical modelWidth < 0 is chosen. Later it uses a negative value to set all the weights to 1, which seems dangerous; wouldn't throwing an exception be safer? It's also not clear to me why 3 is a good default.
There was a problem hiding this comment.
That's a good point. I'll make it raise an exception on negative values, and will add a separate config parameter to turn it on or off.
| "Set to a negative number to disable, which will include all pixels.", | ||
| default=3, | ||
| ) | ||
| useSubfilterMidpoint = pexConfig.Field( |
There was a problem hiding this comment.
Could this be called useMidpoint like in ip_diffim? (Or, alternately, could the one in ip_diffim be called useSubfilterMidpoint? Mostly I'm seeking consistency!)
There was a problem hiding this comment.
I've switched the name to splitSubfilters, and used it consistently throughout.
| Or a placeholder value of 1.0 if ``self.config.modelWidth`` is set to a negative number. | ||
| """ | ||
| if self.config.modelWidth < 0: | ||
| return 1. |
There was a problem hiding this comment.
Again, I don't like this sneaky behavior. If you want a fallback option to have all the weights be 1, that should be an explicit option, not something that happens if the width is set negative.
isullivan
force-pushed
the
tickets/DM-15635
branch
2 times, most recently
from
0dac5d9
to
9a95dfd
Compare
isullivan
force-pushed
the
tickets/DM-15635
branch
from
9a95dfd
to
aa0b13d
Compare
| @@ -470,6 +492,9 @@ def dcrAssembleSubregion(self, dcrModels, bbox, tempExpRefList, imageScalerList, | |||
| The variance of each coadded subfilter image. | |||
| gain : `float`, optional | |||
| Relative weight to give the new solution when updating the model. | |||
| modelWeights : `numpy.ndarray` or `float` | |||
| A 2D array of weight values that tapers smoothly to zero away from detected sources. | |||
| Set to a placeholder value of 1.0 if ``self.config.modelWeightsWidth`` is False. | |||
There was a problem hiding this comment.
| Set to a placeholder value of 1.0 if ``self.config.modelWeightsWidth`` is False. | |
| Set to a placeholder value of 1.0 if ``self.config.useModelWeights`` is False. |
isullivan
force-pushed
the
tickets/DM-15635
branch
from
aa0b13d
to
c0de044
Compare
No description provided.