Add option to write uvw_array as double precision in UVFITS when data_array is single precision #1508
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1508 +/- ##
=======================================
Coverage 99.93% 99.93%
=======================================
Files 63 63
Lines 21834 21849 +15
=======================================
+ Hits 21819 21834 +15
Misses 15 15 ☔ View full report in Codecov by Sentry. |
tests/uvdata/test_uvfits.py
Outdated
| if uvw_double: | ||
| np.testing.assert_allclose(uvd.uvw_array, uvd2.uvw_array, rtol=0, atol=1e-13) | ||
| else: | ||
| assert not np.allclose(uvd.uvw_array, uvd2.uvw_array, rtol=0, atol=1e-13) |
There was a problem hiding this comment.
It would be good to additionally check here that the values are consistent with single precision (in addition to checking that they'd not "double-ish" precision). And any reason not to use rtol here instead of atol (given that it's floating-point precision we're working with)?
bhazelton
force-pushed
the
uvfits_double_uvws
branch
from
dedfda3 to
329a9e9
Compare
| if uvw_double: | ||
| np.testing.assert_allclose(uvd.uvw_array, uvd2.uvw_array, rtol=1e-13) | ||
| else: | ||
| np.testing.assert_allclose(uvd.uvw_array, uvd2.uvw_array, rtol=1e-7) |
There was a problem hiding this comment.
@bhazelton -- I think actually holding on to both checks when uvw_double=False would be good (both that it fails if rtol=1e-13 and passes when rtol=1e-7), since that's a pretty strong indicator that double-precision isn't being used. Or otherwise, adding a check that the UU2/VV2/WW2 entries aren't in there (although I'm not sure if there's a straight-forward way to do that w/ the astropy FITS interface).
There was a problem hiding this comment.
ok, added that one back.
Need to test that this works properly with CASA
bhazelton
force-pushed
the
uvfits_double_uvws
branch
from
329a9e9 to
240ff1c
Compare
Description
Motivation and Context
This adds the option write the uvw_array as double precision in UVFITS even when the data_array is single precision (they are always written as double precision when the data array are double precision). This is done using the same mechanism as is used for the time_array in UVFITS files, namely creating two identically named group parameters that are intended to be added together. This happens automatically in astropy's FITS reader, but it is possible this could mess up other UVFITS readers, so I added an option to allow it to be turned on or off and defaulted it to on because I think most users would prefer not to lose precision on their uvws.
This problem was noted as far back as #154 and it has come up occasionally since then.
Types of changes
Checklist:
New feature checklist: