astropy / photutils

Astropy package for source detection and photometry. Maintainer: @larrybradley
https://photutils.readthedocs.io
BSD 3-Clause "New" or "Revised" License
248 stars 138 forks source link

Have datasets use Quantity with units, or at least test that this works #173

Closed eteq closed 4 years ago

eteq commented 10 years ago

While looking at the notebook @larrybradley made in #160 I noticed the photutils.datasets subpackage. I also noticed that this package does not seem to be using units at all.

I think it would be extremely valuable if the units of the output images at least had the option of appropriate units. E.g., a flux input should actually be units of flux (or at least something that astronomers recognize as flux-like).

It may not make sense to have this forced into the datasets functions, as sometimes you may want adu and other times actual flux units... But I think it would be valuable to make sure this works, and examples should make use of it so users are exposed to how units can make things easier.

A more general question for @larrybradley and @cdeil that may or may not be closely tied to this: I have some code that does something like this for my own work, but it also does things like allowing you to specify extra bits like read noise and telescope aperture, and then makes sure all the units are consistent. Should I try to adapt this into datasets? Or should there be another photutils module along the lines of "simulated images"?

eteq commented 10 years ago

@larrybradley - #174 sort of addresses this, but of course it would be better if it was more tightly integrated. But I see your point that modeling makes this awkward right now. Should we leave this open as a reminder to try to do that when it makes sense? (I.e., when modeling parameters can by quantities) Or should I just close this now?

eteq commented 10 years ago

@larrybradley @cdeil - I posted a gist that shows the sort of thing I'm talking about in the last paragraph of the description - the MockImage class is the main thing I'm thinking of.

larrybradley commented 10 years ago

@eteq It's up to you if you want to leave this open as a reminder.

MockImage certainly is higher fidelity then the simple images produced in datasets. Something like MockImage could be part a separate module (perhaps in imageutils?) to generate "realistic" simulated images or to add synthetic sources to real images (e.g. for completeness studies). That's probably a better place to put more effort. datasets here are mainly for docs and testing.

As an aside, I see in MockImage that you have models for Moffat and Sersic profiles. I was considering adding such models and others (like exponential disk, de Vaucouleurs, etc.) in astropy. Is there some reason why such models aren't in astropy??

cdeil commented 10 years ago

Adding extra models to astropy.modeling would definitely be welcome!

I'm not sure if it's worth to do more fancy image simulation code in photutils or imageutils ... everyone needs something different and will probably write their own script anyways or if they do bother to learn someone else's code, they should use a very feature-rich tool like http://www.astromatic.net/software/skymaker . And there's bigger fish to fry in the next few weeks.

One thing I'd like to have is a generic function to make images of source catalogs where sources are represented by 2-dimensional Astropy models that is fast by only evaluating the models on small cutouts outside of which the model is almost zero instead of the whole image. (Similar to what make_gaussian_sources does, but support more models and only evaluate on cutouts.) I have code that does this for a few models, but I haven't been able to come up with a generic function to do this ... if you have a suggestion of even code to do this I'd be very interested ... there was a discussion on astropy-dev a while ago about optionally adding sampling sets to Astropy models, that would probably do the trick.

cdeil commented 10 years ago

@eteq If you do make a pull request for your image simulation code, I'd personally prefer photutils.datasets, because there's still the plan to put imageutils into the core as astropy.image soon and I think mock image simulation code isn't really core material (or at least should be used for a year to make sure the API is useful and stable).

eteq commented 10 years ago

@cdeil - hmm, maybe only the part that does all the calculations for gain and telescope size and such, then? That is, instead of having it generate the "real" flux, have something in datasets that works like that, except the user has to provide a flux image (with appropriate units)?

My thinking is that this is something everyone should do, but often don't because they don't want to bother with keeping the units straight and such. So we can give that to them but let them do the actual image flux simulation side.

larrybradley commented 4 years ago

Not sure if this is still relevant 6 years later.... :) Feel free to submit a PR to add something like MockImage to create realistic simulated images.