Have Standardizers add obstime to LayeredImage

[wilsonbb]

Fix both theButlerStandardizer and theFitsStandardizer to correctly set the observation time when generating standardized LayeredImages.

Currently, when an ImageCollection creates a WorkUnit, it uses the ButlerStandardizer and theFitsStandardizer implementations of Standardizer.toLayeredImage() which are not correctly calling LayeredImage.set_obstime()

Note that for testing the FitsStandardizer, we used the same test suite as for the KBMODV1 standardizer since it inherits FitsStandardizer.toLayerImage.

[DinoBektesevic]

This will work and since it's my biggest dream ever to get rid of the damned layered and raw image I don't mind a bit of hot-fixes that are in place - just a few questions so that at least I understand what the problem is.

[DinoBektesevic]

Why are we explicitly casting the mask to float32? Is Eigen complaining to you? If so then we should address this in the standardizeMaskImage method.

[wilsonbb]

I just added this because we were doing it in the FITS standardizer and I thought that it might help to catch some edge case.

In ButlerStandardizer.standardizeMaskImage() we do currently explicitly convert to a bool dtype. Do you know if there was any particular reason why we preferred the bool over float32 dtype? Presumably to save on memory but that adds confusion as to why we cast the mask to float32 in other standardizers

[wilsonbb]

Though actually taking another look, in main we do still still get back to a float32, probably in the conversion to Eigen when we create the RawImage

I'm starting to feel that we should remove all the type conversions both in here and instandardizeMaskImage

[DinoBektesevic]

What dtype does Rubin butler return for the mask? I have a feeling I assumed something here and then forgot about it. Mostly astropy.io.fits and the butler should already return float32s. I think Butler might be returning an int for mask, but in that case this cast would still be better in the standardizer I think.

[wilsonbb]

(This comment seems to be about the butler standardizer but is on the FITS standardizer file? Sorry if I'm the one confused here, but answering about the FITS standardizers)

Yeah I saw that you added it in this "Fix bug in Fits Standardizers." commit but I am also not sure what it was you were specifically trying to fix. But overall the FITS standardizer seems fairly broad/general so it seems like it could be fine to do a default conversion of the dtype

Related, we also currently spoof that the mask should be np.int32 here and then in turn explicitly cast the mask dtype to be boolean ion KBMODV1._standardize_mask().

So I guess the overall the question maybe is just what do we want the mask dtype to be since we seem to be a bit inconsistent about it

[DinoBektesevic]

The array is read into the memory by Astropy based on the header keywords DTYPE. It's the same for Rubin. Originally Eigen proposal was to have 3 different arrays (float, double and ints) but that was shut down. This induces a copy and a cast to float, since float is the only array people wanted. There's really no need to have more than int8 for the mask and it would save space on the GPU, but the work to get that done was deemed too much to do at the time.

So this is true for both Rubin and any of the FITS standardizers. Then I floated the casts up to Python by banning the implicit conversion and casts in the Python bindings in C++:

https://github.com/dirac-institute/kbmod/blob/main/src/kbmod/search/raw_image.cpp#L465

But I seem to have a few unnecessary ones. From the code it seems like I wanted to add a constructor onto LayeredImage that takes a single t and propagates it down, but I seem to have forgot or something. Rubin seems to return int masks, other files seems to return whatever they want. This can be fixed and reduce memory and time requirements, but it's nothing that should be addressed in this PR so it's all ok. Just one more thing for the TODO list.