Add an option to use the HERA gridding algorithm to find redundancies

[bhazelton]

Description

This adds a new option for redundancy finding to use the HERA gridding algorithm rather than the existing clustering algorithm. This has some advantages, it's faster and will not throw errors, but it might not handle non-redundant arrays well and the tolerance needs to be well specified to be less than half the minimal antenna spacing.

I also fixed what I am pretty convinced is a bug for data sets that are not rectangular in the basline-times where baselines could be compared at different times. I now use the antenna positions to get ENU baseline vectors if the object is not rectangular in baseline-times. This bug surfaced when I was testing the new HERA algorithm.

Questions for this PR:
We could decide to just replace the existing algorithm with the HERA algorithm rather than making it an option. Or we could default it to the HERA algorithm. I'm open to any of these options. I haven't yet updated the changelog because I wasn't sure which direction we wanted to go.
We decided to make the new method the default but with a warning message that the default had changed.

Motivation and Context

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation change (documentation changes only)
• Version change
• Build or continuous integration change

Checklist:

• I have read the contribution guide.
• My code follows the code style of this project.

Bug fix checklist:

• My fix includes a new test that breaks as a result of the bug (if possible).
• All new and existing tests pass.
• I have updated the CHANGELOG.

New feature checklist:

• I have added or updated the docstrings associated with my feature using the numpy docstring format.
• I have updated the tutorial to highlight my new feature (if appropriate).
• I have added tests to cover my new feature.
• All new and existing tests pass.
• I have updated the CHANGELOG.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b448881) 99.92% compared to head (2db05bb) 99.92%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1403   +/-   ##
=======================================
  Coverage   99.92%   99.92%           
=======================================
  Files          37       37           
  Lines       20767    20807   +40     
=======================================
+ Hits        20751    20791   +40     
  Misses         16       16           

Files	Coverage Δ
pyuvdata/utils.py	100.00% <100.00%> (ø)
pyuvdata/uvdata/uvdata.py	100.00% <100.00%> (ø)

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b448881...2db05bb. Read the comment docs.

[jpober]

@baron-de-montblanc can you take a look at this code and see if it works any better (or worse) on MWA Phase 2? It may simplify redundancy finding (i.e. no need to throw out the core antennas first).

[bhazelton]

It won't error on core antennas, but it could inappropriately group them in with the hex tiles. The tolerance parameter will be key to that, I haven't messed with this much for non-redundant arrays.

[baron-de-montblanc]

@baron-de-montblanc can you take a look at this code and see if it works any better (or worse) on MWA Phase 2? It may simplify redundancy finding (i.e. no need to throw out the core antennas first).

Yes, this works well for my Phase 2 data. I set tol=3 (the minimum antenna spacing in the file was 8m) and it didn't throw any errors. In total it got rid of about 1/3 of the baselines. Interestingly enough, after the initial compression, I ran it a few more times, gradually increasing the tolerance. I increased until tol=1000 and no additional baselines were thrown out (and no errors thrown either).

[bhazelton]

Yes, this works well for my Phase 2 data. I set tol=3 (the minimum antenna spacing in the file was 8m) and it didn't throw any errors. In total it got rid of about 1/3 of the baselines. Interestingly enough, after the initial compression, I ran it a few more times, gradually increasing the tolerance. I increased until tol=1000 and no additional baselines were thrown out (and no errors thrown either).

great! Thanks for the report.

[bhazelton]

@jpober I think the biggest question I have is should we just replace the old clustering method with this HERA-based gridding method or should we keep them both around? And if the latter, what should the default method be. I'm sort of leaning towards just replacing it but I haven't thought about this super deeply.

[jpober]

I don't have a strong opinion. I'm slightly reluctant to remove functionality until we have dug a little deeper and confirmed the results are (dare I say) redundant. That said, the redundancy finder was already a little confusing since we had calculations using uvws and antenna positions as separate options. Adding a third sounds like too much. So I'm ok with just replacing it, but maybe add a warning that there's been an update and that it's not guaranteed to reproduce older results?

[bhazelton]

On the telecon we decided to make the new method be the default but with a warning that the default has changed.

[bhazelton]

@jpober I made the change we discussed in the telecon yesterday -- I defaulted it to use the HERA algorithm but with a warning.

[jpober]

I'm all for giving credit to the HERA team, but do we want the warnings to refer to this as the "HERA gridding based algorithm"? I feel like for the average user this might sound like it's the algorithm for use with the HERA array, not a general algorithm from the HERA team. Maybe we could say "Defaulting to True to use the new gridding based algorithm (developed by the HERA team) rather than the older clustering based algorithm" or something?

[bhazelton]

I'm all for giving credit to the HERA team, but do we want the warnings to refer to this as the "HERA gridding based algorithm"? I feel like for the average user this might sound like it's the algorithm for use with the HERA array, not a general algorithm from the HERA team. Maybe we could say "Defaulting to True to use the new gridding based algorithm (developed by the HERA team) rather than the older clustering based algorithm" or something?

Yep, sounds good. I'll make that change.

[jpober]

Looks good. Can you confirm that the error in the build against HERA Cal is expected? If so, I'll go ahead and merge.