desihub / desitarget

DESI Targeting
BSD 3-Clause "New" or "Revised" License
18 stars 23 forks source link

add units to target and truth files #237

Closed sbailey closed 6 years ago

sbailey commented 6 years ago

From @weaverba137 in desihub/two_percent_DESI#6:

The truth and targets files don't set units on any values, even obvious ones like RA, DEC.

This applies to both the output of select_targets and (mpi_)select_mock_targets. For the mocks, units could be added during the merging by join_mock_targets but it would be better for them to be added in the original per-pixel files that are written.

geordie666 commented 6 years ago

Can we clarify what "set" means in this sense. In the header of the output fits files?

weaverba137 commented 6 years ago

Specifically, the TUNIT FITS header. For example RA, Dec are measured in degrees. An example from another file:

TTYPE9  = 'RA      '           / label for field   9
TFORM9  = 'D       '           / data format of field: 8-byte DOUBLE
TUNIT9  = 'deg     '           / physical unit of field
TTYPE10 = 'DEC     '           / label for field  10
TFORM10 = 'D       '           / data format of field: 8-byte DOUBLE
TUNIT10 = 'deg     '           / physical unit of field

The target and truth tables contain fluxes, which need units too (nanomaggies, I assume).

Caveat, I think at least some FITS readers/writers complain about nanomaggies as a non-standard unit.

moustakas commented 6 years ago

I can do this in my working branch of mock.build.empty_targets_table. Indeed, nanomaggies trigger a complaint from FITS readers/writers but presumably we must forge ahead. Astropy also throws a warning with dex as a unit (e.g., log-solar metallicity, which is stored for the LRG templates).

Also, I haven't fully tested this yet but with astropy v2.x I get an exception when I try to read/write an astropy table in which one or more columns do not have unit at all and I have not yet figured out how to turn this off (since some quantities obviously will be unitless).