search

HERA-Team / hera_cal

Library for HERA data reduction, including redundant calibration, absolute calibration, and LST-binning.
MIT License
13 stars 8 forks source link

feat: add more robust RedundantGroups class #887

Closed steven-murray closed 1 year ago

steven-murray commented 1 year ago

This just adds a new RedundantGroups class that does a very similar thing to the RedDataContainer except that it doesn't worry about actual data, i.e. it's just a management class for the redundant baseline groups themselves. I've added an attribute to the RedDataContainer that points to one of these objects, which stands in as the redundant groups of the array rather than the data itself.

Some points I noticed on the way:

  1. The test_redcal_run test was failing on my own system in this PR, and I thought I'd screwed something up, but I tested it on main as well and it failed. That same test is one that randomly fails in linsolve sometimes, so I think there's something brittle about it that I don't understand.
  2. I think the fact that the RedDataContainer is weirdly mixing data keys and array keys is making some things not work. By that, I mean that the key by which it indexes a redundant group is either the first bl in the group that also exists in the data, if such a key exists, OR it's just the first in the group. This is going to mean that when you read different files and set the keys by which redundant averaged data is indexed, they can be different. I think it might make more sense to be a bit more clear about this distinction in the class.
codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.04 :tada:

Comparison is base (a3aebb3) 97.15% compared to head (1c0bcab) 97.20%.

:exclamation: Current head 1c0bcab differs from pull request most recent head cc27557. Consider uploading reports for the commit cc27557 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #887 +/- ## ========================================== + Coverage 97.15% 97.20% +0.04% ========================================== Files 20 21 +1 Lines 9214 9362 +148 ========================================== + Hits 8952 9100 +148 Misses 262 262 ``` | [Impacted Files](https://codecov.io/gh/HERA-Team/hera_cal/pull/887?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=HERA-Team) | Coverage Δ | | |---|---|---| | [hera\_cal/datacontainer.py](https://codecov.io/gh/HERA-Team/hera_cal/pull/887?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=HERA-Team#diff-aGVyYV9jYWwvZGF0YWNvbnRhaW5lci5weQ==) | `98.36% <100.00%> (-0.04%)` | :arrow_down: | | [hera\_cal/red\_groups.py](https://codecov.io/gh/HERA-Team/hera_cal/pull/887?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=HERA-Team#diff-aGVyYV9jYWwvcmVkX2dyb3Vwcy5weQ==) | `100.00% <100.00%> (ø)` | | | [hera\_cal/redcal.py](https://codecov.io/gh/HERA-Team/hera_cal/pull/887?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=HERA-Team#diff-aGVyYV9jYWwvcmVkY2FsLnB5) | `98.33% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=HERA-Team). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=HERA-Team)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.