tickets/DM-20129 #9
Conversation
| # If the source is too faint for background detection, | ||
| # initialize it as a PointSource | ||
| pass | ||
| if not initialized: |
There was a problem hiding this comment.
Should this happen if the pointSource is set to False?
There was a problem hiding this comment.
I see the above comment, but am just wondering the logic, should there not just be an error thrown or something?
There was a problem hiding this comment.
Part of this might be the nomenclature. Both ExtendedSource and PointSource are used to model extended sources, they just differ in their initialization. An ExtendedSource uses a multiband coadd to attempt to initialize large extended sources but requires a certain signal to noise level to function properly. Very faint sources barely above the background will not have enough flux to meet this requirement, so we initialize them as a point source but they are still allowed to grow.
| # also using scipy.signal.convolve as a sanity check | ||
| morphs = np.array([scipy.signal.convolve(m, targetPsfImage, method="direct", mode="same") | ||
| for m in morphs]) | ||
| morphs /= morphs.max() |
There was a problem hiding this comment.
This may just be me reviewing quickly without thinking through too much, but do you want the max or the sum in the normalization
There was a problem hiding this comment.
The max. This seems to be a more stable normalization, most likely because the center of most objects is better behaved. Recall that the value of a pixel in the model is the product of its SED and the morphology in that pixel. Since the center of an object has a higher S/N it is easier for the optimization algorithm to correctly fit the center and takes more time to fit the outside of a source. If we instead use a normalization of the entire morphology then fiddling with flux on the outskirts of an object also adjusts the flux in the center (due to the normalization of all the pixels) and takes longer to converge.
| psfImages /= psfImages.sum(axis=(1, 2))[:, None, None] | ||
|
|
||
| channels = range(len(images)) | ||
| return targetPsfImage, psfImages, images, channels, seds, morphs, targetPsf, psfs |
There was a problem hiding this comment.
When you have thing many parameters consider returning a pipe_base.Struct, collections.namedtuple, or types.SimpleNamespace so that it is easier to reason and work with so many objects.
There was a problem hiding this comment.
I know what you mean but I'm not a big fan of returning a new class just for a return value.
Add unit tests to ensure the package is working and to track changes in a simple blend.