Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add units to target and truth files #237

Closed
sbailey opened this issue Nov 14, 2017 · 3 comments · Fixed by #264
Closed

add units to target and truth files #237

sbailey opened this issue Nov 14, 2017 · 3 comments · Fixed by #264
Assignees

Comments

@sbailey
Copy link
Contributor

sbailey commented Nov 14, 2017

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
Copy link
Contributor

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

@weaverba137
Copy link
Member

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
Copy link
Member

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants