HERA-Team / hera_sim

Simple simulation code for HERA-like redundant interferometric arrays
Other
16 stars 8 forks source link

fix: don't simulate xtalk for autos #202

Closed r-pascua closed 2 years ago

r-pascua commented 2 years ago

I forgot to make sure that the new cross-coupling model is only applied to cross-correlations. This has been fixed.

codecov[bot] commented 2 years ago

Codecov Report

Merging #202 (cfc295c) into main (e9f82ab) will increase coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #202   +/-   ##
=======================================
  Coverage   96.58%   96.59%           
=======================================
  Files          24       24           
  Lines        2785     2787    +2     
=======================================
+ Hits         2690     2692    +2     
  Misses         95       95           
Impacted Files Coverage Δ
hera_sim/sigchain.py 98.41% <100.00%> (+0.01%) :arrow_up:

Continue to review full report at Codecov.

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

aewallwi commented 2 years ago

I forgot to make sure that the new cross-coupling model is only applied to cross-correlations. This has been fixed.

What's the reasoning to apply this only to cross correlations? I think that the autos might be a good place to check for over-the-air cross-talk effects as well.

r-pascua commented 2 years ago

@aewallwi this model is only valid for the cross-correlations. The autos would need a different model. In particular, the contribution to the visibility from cross-coupling would be the sum of all of the cross-correlations, with each cross-correlation scaled by an appropriate choice of reflection coefficient. The model here would effectively rescale the input auto to (1 + eps) times itself, with eps being the reflection coefficient for the particular antenna. In the previous validation effort, we also excluded cross-coupling from the autos, since it's effectively a second-order effect.

For reference, see Equation 12 in HERA Memo 64.

aewallwi commented 2 years ago

@r-pascua Ah! Thanks for clarifying! I thought this was for the Josaitis cross-talk model. This is definitely fine with me!

steven-murray commented 2 years ago

Oh add to the changelog :-)

r-pascua commented 2 years ago

Thanks for the reminder! I will log the changes.