Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Corr metric overhaul #393

Merged
merged 109 commits into from
Mar 3, 2021
Merged

Corr metric overhaul #393

merged 109 commits into from
Mar 3, 2021

Conversation

dstorer
Copy link
Contributor

@dstorer dstorer commented Feb 1, 2021

Add correlation metric and cross-polarization metric to use for antenna flagging. The correlation metric is measures how well antennas are correlating with each other, and the cross-polarization metric compares this value between the same-pols and different-pols to identify crossed antennas. See the daily notebooks for visuals of this metric and the cross-polarization metric.

The correlation metric is calculated for each polarization as follows:
Screen Shot 2021-02-10 at 10 32 17 AM

When only sum visibilities are provided (e.g. for H1C), then interleaved integrations are used to calculate the evens and odds.

Additionally, we've stripped out the unneeded old functionality for Mean Vij based metrics and the cross-polarization detection based on Mean Vij. The former is superseded by auto_metrics, the latter is superseded by the new cross-correlation based detection of cross-polarized antennas. We've also removed all modified z-score based antenna removal in favor of strict cuts.

jsdillon added a commit to HERA-Team/hera_cal that referenced this pull request Feb 11, 2021
This function is being eliminated in HERA-Team/hera_qm#393 and since it's only used in hera_cal, it should live there.
jsdillon added a commit to HERA-Team/hera_cal that referenced this pull request Feb 13, 2021
This function is being eliminated in HERA-Team/hera_qm#393 and since it's only used in hera_cal, it should live there.
@dannyjacobs
Copy link
Contributor

Thanks @dstorer for your explanation, they made things clearer. My revised notes (here, now more? readable) try to expand on and and justify the proposed change. Things are close. I see two main questions.

Q1
In your equation 3 you write even_ij / | even_ij|. Do you really mean to divide each visibility spectrum by its abs? Is that maybe a typo? That operation would cancel the entire amplitude of the fringe. It’s hard to predict what that would look like. I was expecting you to divide by the autos. This would cancel out the gain terms to get back the “true” visibility (see my notes).

Q2
In your pasted writeup you describe a process for computing a cross-pol metric based on cross correlations then below you say that cross-polarization detection has been preceded by auto_metrics. Which is being used to ID cross pols? The method using cross poles is has many steps and summations making it difficult to predict what the output ought to be, a method that estimates the polarization fraction in the auto would be far vastly more preferable.

@dstorer
Copy link
Contributor Author

dstorer commented Feb 17, 2021

@dannyjacobs in response to your 2 questions:

1: no, that's not a typo, the idea behind that choice is exactly that the amplitude of each baseline is normalized to 1, so if the phases are noise-like this value will average down to zero, but if the antennas are well correlated then the phases should not be noise-like, and this value should average to 1. In the very beginning of developing this plot I was normalizing by the autos, but found that when I normalized the way described above it did a better job at highlighting when nodes weren't correlating and made the metric somewhat less sensitive to baseline length. I agree that this is a fundamentally different metric, but what we're currently doing seems better motivated to me (and we've seen it work for a long time now).

  1. I think there was a small typo in that last sentence that made it confusing - auto_metrics is not identifying cross-polarized antennas, that is being done using the cross pol metric that is based on the correlation metric. The reason for switching away from identifying cross-polarized ants based on relative power in the autos is that we observed that method was not very robust, and was very sensitive to antennas with one polarization that was completely dead - while this is still a problem, it is a distinctly different problem than polarization cables being swapped. The new metric described above is much more reliable in catching antennas that are actually crossed, rather than low power or dead in one or both pols.

I'm not sure I 100% followed your write-up, but it seems like aside from these 2 points we are basically on the same page. Let me know if you have more questions.

@jsdillon
Copy link
Member

On 1: Dividing by the amplitude has the benefit of making the RFI largely irrelevant without having to use any medians over frequency or time. Part of what I really like about this metric, now that we have auto_metrics as a complement, is that it's really focused on what information the phases of the visibilities can give us about the health of the array, compared to auto_metrics which (by necessity) only looks at amplitudes.

On 2: Regarding "The reason for switching away from identifying cross-polarized ants based on relative power in the autos" we actually used the crosscorrelations, not just the autos, to identify cross-polarized antennas before. But the MeanVij-based metrics were still just looking at visibility amplitudes, including the one for identifying crosses (which looked for larger amplitudes in en/ne than in nn/ee).

jsdillon added a commit to HERA-Team/hera_pipelines that referenced this pull request Feb 17, 2021
This is designed for the new corr-based ant_metrics with absolute cuts (see HERA-Team/hera_qm#393)
@jsdillon
Copy link
Member

As a confidence-building measure (and as discussed on today's Analysis/QM telecon) I ran a whole day (2459122) through the new ant_metrics (without any a priori antenna flags). Here's a section of the result from the in-development summary notebook:

image

Looks like we're pretty consistently finding dead and/or crossed antennas. There are other pathologies which auto_metrics picks up that ant_metrics doesn't, but this is very promising.

Copy link
Contributor

@dannyjacobs dannyjacobs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small documentation requests. Only top level thing is to add a CHANGELOG file to the root hera_qm/ directory with the following.

Changes metric approach for correlations and cross_polarizations. TBD memo in draft see issue #395

  • This is a breaking change which removes many functions
  • remove meanvij_metrics, antpol_metric_sum_ratio, per_antenna_modified_z_scores, mean_vij_cross_pol_metrics,
  • add cross_polMetrics, calc_corr_stats
  • many changes to AntennaMetrics object and hidden functions
  • many test changes as necessary

hera_qm/ant_metrics.py Outdated Show resolved Hide resolved
hera_qm/ant_metrics.py Show resolved Hide resolved
hera_qm/ant_metrics.py Show resolved Hide resolved
hera_qm/ant_metrics.py Show resolved Hide resolved


def mean_Vij_metrics(abs_vis_stats, xants=[], pols=None, rawMetric=False):
"""Calculate how an antennas's average |Vij| deviates from others.
def corr_metrics(corr_stats, xants=[], pols=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the description memo is posted, link to it in the docstring. Tracked in issue #395 .

hera_qm/ant_metrics.py Show resolved Hide resolved
hera_qm/ant_metrics.py Show resolved Hide resolved
@jsdillon
Copy link
Member

jsdillon commented Mar 3, 2021

We good to go on this @dannyjacobs? I'd prefer to stop running on this branch on site, if possible.

@jsdillon jsdillon merged commit b1bbfcc into master Mar 3, 2021
@jsdillon jsdillon deleted the corr_metric_overhaul branch March 3, 2021 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants