EoRImaging / FHD

Fast Holographic Deconvolution
BSD 2-Clause "Simplified" License
20 stars 10 forks source link

Min/max baseline comparisons now double #248

Closed nicholebarry closed 3 years ago

nicholebarry commented 3 years ago

Previously, we would cast our baseline calculations to floating point. However, where the cast happened would change the result. This showed up as a difference in the number of visibilities going into the even/odd cubes due to an inconsistent minimum baseline (see #242).

I've modified the code so that the baseline calculations (i.e. calculating the min/max baseline in the obs structure, calculating the baselines in gridding/degridding) so that they are done natively at double precision. I did not notice a change in mem/time requirements, but it should be something to be paid attention to for extreme baseline numbers (perhaps for the SKA?). The number of visibilities are now the same between even/odd cubes.

Recent gridding pushes were already at double precision.

nicholebarry commented 3 years ago

Tested many times over, and I get a nice agreement on the number of visibilities in even/odd cubes for multiple pointings/obs/etc.

I battled with one small error for quite some time, which turned out to be a casting bug. The value in vis_n_full was a long integer, and so dividing it by a float would result in losing a baseline or two at the end of the list. This only was used in high memory situations in the degridding, so a few baselines at approx less than 10 wavelengths were affected.

This is the result before the bug fix: fhd_nb_model_gaussdecomp_update_nofreqdep_corrmin_double_uvf_1061316296_noimgclip_dft_averemove_swbh_dencorr_2dkpower_orig

And after the bug fix: fhd_nb_model_gaussdecomp_update_nofreqdep_corrmin_double_uvf_1061316296_noimgclip_dft_averemove_swbh_dencorr_2dkpower

bhazelton commented 3 years ago

That oscillatory pattern in the pre-fix spectra is fascinating. I'm a little disturbed by the diagonal line in the window in the post-fix spectra. Is that a known artifact? It looks like something Adam found years ago, but I thought we'd killed that one.

nicholebarry commented 3 years ago

You are 100% correct! It's the "ghost line," come back to haunt us again.

Over the years we've backed off the extreme overresolution of the beam because of the smart beam interpolation code that Ian wrote. But it looks like I've found the floor again. Increasing the resolution of the beam moves the line about, as expected.

bhazelton commented 3 years ago

Fascinating! I guess I should be glad, it means the floor is dropping :-P.