dendrograms / astrodendro

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

Remove flux and luminosity attributes for now, and finish renaming analysis properties #54

Closed astrofrog closed 11 years ago

astrofrog commented 11 years ago

Until we figure out how to properly compute the flux and luminosity of structures (as discussed in #47) I think we should remove these properties, which is done in this PR. Once we agree on how to deal with fluxes, we can re-implement them (but better start off without an implementation than with an implementation that could be wrong in some cases)

In addition, I have renamed the properties I had not renamed before, and I think the full set of properties has more consistent and clear names. This addresses #48.

@ChrisBeaumont - could you review this?

astrofrog commented 11 years ago

@ChrisBeaumont - I'm adding a module (flux.py) where we can code up all the conversions between flux units, then will re-instate the flux property. I'm going to continue working on it, but does this approach seem ok to you?

astrofrog commented 11 years ago

For any non-supported units, is it better to crash, or simply to return the value in the same units as the input?

ChrisBeaumont commented 11 years ago

This approach sounds fine by me. I say raise an exception if unsure

astrofrog commented 11 years ago

@ChrisBeaumont - another question, should the pp_catalog and ppvi_catalog functions output the flux by default? Should they always require units to be passed? Or do we simply not output the flux column (and emit a warning) if units were not passed?

astrofrog commented 11 years ago

@ChrisBeaumont - I've spent some more time working on this and I think it's ready for review and real-life testing.

I've re-instated the flux calculation, and at the moment the catalog functions require the flux units to be passed since there is no sensible default. At the moment, only monochromatic fluxes (per frequency or wavelength) per pixel and per beam are supported. As time goes on, we can add more flux conversions (e.g. with K).

I've also made it so that now warnings/exceptions are only shown if the default value is actually being used for something, rather than at the start of the cataloguing routine. For example, we only care about the default beam if converting something from Jy / beam (in practice this doesn't happen because there is no sensible default beam size so an exception would be raised).

Note that at the moment, there is a small approximation, which is that the wavelength of the emission is constant over a cube, i.e. that the velocities are such that the wavelength doesn't change enough to affect the meaning of monochromatic flux units.

ChrisBeaumont commented 11 years ago

On first read, this looks like a definite improvement, and "safer" with regards to unit checking.

I think the flux interface is fine. I think we should maybe provide one or two conversion functions to/from K in the Rayleigh Jeans approximation and Jy / solid angle / hz or whatever it is. That is, even though we don't yet want to try to figure out how to handle K units inside the cataloging routines, we should allow the user to manually convert between units (with the assumption that s/he is doing this carefully, if invoking a specific conversion function).

astrofrog commented 11 years ago

Oh I forgot to mention we do already have MJy/sr (and equivalents) already too. I can try and add support for K. I need to make sure I understand it properly, since I know it also involves beams. Would we assume that if it's in K it is an antenna temperature?

astrofrog commented 11 years ago

It isn't clear to me how we can distinguish between e.g. brightness temperature and antenna temperature. Do you have any suggestions? Or should we just assume antenna temperature, and then emit a warning to let users know of the assumption?

ChrisBeaumont commented 11 years ago

I may not be thinking this all the way through, but:

I think the "common" use case (that is, what I use, and what I've seen radmc 3d use) is a K that is synonymous with flux density under the Rayleigh Jeans approximation. That is, the temperature that satisfies B_nu = 2 nu^2 k T / c^2.

As far as I understand the Kutner and Ulich (1982?) paper, this is the number that a properly calibrated antenna temperature refers to. In practice, you usually need to apply an efficiency correction first (this is the difference between Ta, Ta*, and all that nonsense).

I'm not suggesting we try to apply this final correction automatically. Instead, I'm suggesting that, if a user knows s/he has a pre-calibrated datacube in K, we should make it easy to work with this data unit. Either we:

Option 1 sounds better from a unit safety perspective -- users are more likely to feed in the "wrong" / uncalibrated antenna temperature to the catalog functions, if they don't have to think about it.

Does that make sense / is it wrong?

astrofrog commented 11 years ago

Ok, so you mean that given that this is a special ambiguous case, we treat it differently to the other cases? If so, then I agree that a convenience function makes sense. We could always do option 2 and emit a warning too?

To your question 'is it wrong', my answer is that I have no idea :) I think that maybe we should delay this feature and speak to a few people who specialize in radio data to make sure that we get it right? I don't think this is an urgent feature in the sense that if they can properly calibrate the brightness temperature, they should also simply be able to convert it to one of the supported types of units?

low-sky commented 11 years ago

There are >1 naming conventions for these quantities, alas. In mm-wave astronomy where we get most of our data from, I usually see the convention that there are four temperatures

Tr -- The radiation temperature which is equivalent to the radiation flux density that would be seen in space. Tmb -- The main beam temperature which is ηff Tr where ηff is the filling factor of the emission in the beam. Usually taken to be 1 because of ignorance. TA -- Antenna temperature which is the power coming into the antenna as if feed were replaced by a resistor at temperature TA TA* -- The correction of TA for atmospheric opacity.

In the simple case, ηmbTmb = TA* where ηmb is the beam efficiency. You can break up the main beam efficiency further but this usually doesn't introduce another possible temperature scale.

In practice, the temperatures can be on TA (usually), Tmb (sometimes), TA* (not so often, except for JCMT data). Rarely do you quote Tr since that requires knowing the unknown distribution of emission (spatially and temperature-wise) within your resolution element. If you are lucky, the best convention is to see a header line with a comment indicating what scale:
BUNIT= 'K ' / Tmb scale Given all this, I'd say K unit support is possible, but there could be an opportunity to scale results by a constant to change between temperature scales. I'm clearly not fit to comment about how to do this, but carrying along a string or something might help.

Finally, aren't fluxes in units of Jy Hz and flux densities in units of Jy? Probably painful to carry all this around in variable names.

astrofrog commented 11 years ago

@low-sky - thanks for your comments regarding the temperature! Given this, I agree with @ChrisBeaumont that the best option is to provide a separate utility function to pre-convert data in K to sensible and unambiguous units, with a keyword argument for the different conventions (I opened a new issue for this: #57)

astrofrog commented 11 years ago

@ChrisBeaumont - if Travis passes, shall we merge this and address the issue of the K units in #57?

astrofrog commented 11 years ago

The only question is whether I should change major_sigma and minor_sigma and position_angle so they include the sky_ prefix?

ChrisBeaumont commented 11 years ago

The more succinct names are fine by me, since the docstrings are clear. Fine to merge by me

astrofrog commented 11 years ago

Ok, thanks!