Precompute __antpair2ind_cache

[r-pascua]

Validating the ModelData object can be needlessly expensive when setting up simulations at the scale of full HERA because of the check that only one of (ai, aj) or (aj, ai) show up for any antenna pair. This workaround can avoid the problem by precomputing the antenna pair to baseline-time index cache.

Unfortunately, after writing and testing this, I have come to the realization that we can probably speed up the code block in the validation section (e.g., here) with something like

antpairs = set(uvdata.get_antpairs())
if any((aj,ai) in antpairs for (ai,aj) in antpairs if ai != aj):
    raise ValueError(...)

instead of checking that either uvdata.antpair2ind(ai,aj) or uvdata.antpair2ind(aj,ai) returns None. The only corner case that I think I can see where this fails would be something like the "nontrivial" case in my tests that exclude setting the blts_are_rectangular attribute to False (while also having time as the fast part of the baseline-time axis). Although I think that case would fail silently, as it would ultimately tie back to uvdata.get_antpairs() not returning the full list of antenna pairs.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.49%. Comparing base (14a8080) to head (798ed2b).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #420      +/-   ##
==========================================
+ Coverage   93.47%   93.49%   +0.02%     
==========================================
  Files          25       25              
  Lines        3371     3384      +13     
  Branches      562      566       +4     
==========================================
+ Hits         3151     3164      +13     
  Misses        115      115              
  Partials      105      105              

Flag	Coverage Δ
unittests	93.49% <100.00%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[steven-murray]

Thanks @r-pascua --- yeah I think your solution in the OP is the easiest/best way to go. You're right that it would fail in that case, but that case is INVALID DATA anyway (unfortunately, it's possible to get into situations where data is invalid in pyuvdata -- this is due to the ability to update attributes in-place, which is always a tricky thing to manage).

[r-pascua]

In that case, I figure I should just implement the suggested change in the OP and add some tests to ensure that it works for valid UVData configurations? I'm wondering about keeping the new function for precomputing the cache, since precomputing the cache should make MatVis._reorder_vis much faster in the case where we actually need to reorder the output from matvis (I'm guessing that the slow part in that function is also the repeated calls to antpair2ind). On a related note, I think we can use the lessons learned from this to also handle the blts_are_rectangular and time_axis_faster_than_bls case in MatVis._reorder_vis—should I add that feature as well?

[steven-murray]

Yeah -- I reckon use your more simple example to fix this. If there's another place we can use the cache, let's use it there! The reorder_vis function is stupidly slow (when it's not short-circuited).