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

Fix mutual coupling conjugation #268

Merged
merged 3 commits into from
Jan 18, 2024
Merged

Fix mutual coupling conjugation #268

merged 3 commits into from
Jan 18, 2024

Conversation

r-pascua
Copy link
Contributor

@r-pascua r-pascua commented Aug 8, 2023

This PR fixes the conjugation error in the electric field propagator in the mutual coupling simulator. This should now ensure that the fringe-rate vs delay waterfalls for simulated coupling have the same shape as in the data.

Also fix the test to account for the updated electric field propagator
convention.
Re-ran the tutorial notebook with the updated conjugation convention,
and updated one figure to explicitly show which "blob" corresponds to
which visibility, as presented at the 2023 HERA Busy Week.
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Contributor

@steven-murray steven-murray left a comment

Choose a reason for hiding this comment

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

This looks good, thanks @r-pascua. Figures in the notebook look great!

@r-pascua
Copy link
Contributor Author

This took a long time to come around to, but I've finally made a decisive comparison of the coupling simulations against what we see in the data. The amplitude of the coupling is greater in the data than what is predicted by the simulation, but it looks like the shapes in fringe-rate vs delay space agree. I figured I would post the results here for reference.

Screenshot from 2024-01-17 19-57-11

Copy link

codecov bot commented Jan 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (35c472b) 92.92% compared to head (40b3491) 92.92%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #268   +/-   ##
=======================================
  Coverage   92.92%   92.92%           
=======================================
  Files          24       24           
  Lines        3223     3224    +1     
  Branches      705      705           
=======================================
+ Hits         2995     2996    +1     
  Misses        126      126           
  Partials      102      102           
Flag Coverage Δ
unittests 92.89% <100.00%> (+<0.01%) ⬆️

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

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

@r-pascua r-pascua merged commit e7d7ee1 into main Jan 18, 2024
11 checks passed
@r-pascua r-pascua deleted the fix_conjugation branch January 18, 2024 01:48
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.

2 participants