DM-16347: Fix modelWeights bugs. #234
Conversation
| weights = np.zeros_like(maskedImage.image.array) | ||
| bboxGrow = afwGeom.Box2I(bbox) | ||
| bboxGrow.grow(self.bufferSize) | ||
| bboxGrow.clip(dcrModels.bbox) |
There was a problem hiding this comment.
It took some back-and-forthing to figure out the need for and significance of bboxGrow. For the latter, some documentation or comment would be nice. For the former, I see that this weight image is used in the context of a similarly bboxGrown image in the dcrAssembleSubregion() function at L521. Given that these are required to be the same, it is a bit uncomfortable that they are computed independently in each function. Might it be useful to set bboxGrow in run itself and pass this directly into both functions (calculateModelWeights() and dcrAssembleSubregion(), and I do realize the latter will also need the subs for the dcrModels.assign call at L528, but it still may be worthwhile for clarity)? That way the place to document it would naturally be part of the Parameters lists.
Regardless, you also need to update the docs for this function. It would also be nice to have a bit more info on what the weights here actually represent (just calling them "[model] weight values that tapers smoothly to zero away from detected sources" is not entirely illuminating...but that is out of scope for this ticket, so is up to you!)
There was a problem hiding this comment.
Good suggestions! I have renamed bboxGrow to dcrBBox and moved the definition up to the run method, so that it can be passed in to dcrAssembleSubregion and calculateModelWeights. Then I can remove the duplicated from both those places.
I also updated the documentation.
isullivan
force-pushed
the
tickets/DM-16347
branch
from
38012e8
to
90eae14
Compare
Some cases errored with mis-matched bounding boxes, and assembling a coadd returned early if the new option
useModelWeightswas turned off.