dendrograms / astrodendro

Generate a dendrogram from a dataset
https://dendrograms.readthedocs.io/
Other
38 stars 38 forks source link

Review luminosity calculation #47

Open astrofrog opened 11 years ago

astrofrog commented 11 years ago

The luminosity calculation should take into account that flux units might be per pixel or per unit area, and I don't think we should default to a given pixel scale since there is no sensible default - we should just raise an exception.

astrofrog commented 11 years ago

So here's the issue - I don't think we can find the luminosity without having units for the flux and without a distance, so how do we deal with this if the meta-data is not supplied? I don't think it makes sense to assume defaults for these, so do we simply make them into required meta-data for those methods? (which makes them required meta-data for the *_catalog functions. Alternatively, the *_catalog functions can skip properties they can't compute if meta-data is missing?

low-sky commented 11 years ago

Radio units are a wretched mess with no sensible default. In cprops we required a lot of metadata just to get a set of CO (1-0) all converted to a single unit. Since this is more general, I think you shouldn't try to wrangle with Luminosity and perhaps use that as an example of conversion after computation. Alternatively, you could try to support specific BUNITs and then just throw a warning and not compute if the BUNIT isn't one of the acceptable few. Even supporting K is problematic since it requires Beam information which is reported several ways.

astrofrog commented 11 years ago

@low-sky - I like the idea of supporting flux (and by extension luminosity) only for a subset of units. Things get even hairier when taking about cubes, right? The question is then how we should proceed with the automated cataloguing routines if they are not able to compute the flux or luminosity - should we output only quantities that can be computed? Or raise an error?

There are several quantities we can always compute even with no meta-data (which just means values will be in data units/pixel coordinates):

Maybe we should just stick to these by default? Then we could have a keyword argument for the catalogs which would be flux=False by default which could then be turned on to compute the actual flux if the appropriate meta-data are passed.

ChrisBeaumont commented 11 years ago

I like this discussion. Whatever approach we decide on, I'm +1 on aiming on the side of explicit / foolproof. As @low-sky says, all the various radio temperature scales are maddening, and rarely are tools very helpful in computing properties in the correct units. The more that the dendro code can provide helpful error messages, the better.

Maybe we should be even more strict with how BUNIT is specified for radio data? We could define some new astropy units + equivalencies for the various temperature scales, and mandate that users choose a specific temperature subtype (e.g. main beam) and possible efficiency correction. As I understand from @taldcroft's scipy talk, it is possible to register new astropy units from the outside.

Design-wise, I'd like to figure out how to move as much of this input checking logic into isolated functions (i.e. I don't want lots of branches inside the *Statistic classes). Decorators that specify metadata constraints might work nicely.

astrofrog commented 11 years ago

Thanks for the feedback! I'll work on this all this week to see if there is a nice way to handle all this. Having code to deal with flux units for radio cubes would be very handy (and can be kept in a separate file).

astrofrog commented 11 years ago

In the spirit of http://www.youtube.com/watch?v=OrpPDkZef5I, I've removed flux and luminosity in #54 and then plan to re-implement it later for a subset of flux units (but this needs to be done carefully and case by case).