maint: update API for new multi-processing fftvis

[steven-murray]

This PR updates the FFTVis API so that it works with the multiprocessing-enabled fftvis.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[codecov]

Codecov Report

Attention: Patch coverage is 94.08602% with 11 lines in your changes missing coverage. Please review.

Project coverage is 93.42%. Comparing base (e0b4001) to head (e6458a3).
Report is 20 commits behind head on main.

Files with missing lines	Patch %	Lines
hera_sim/visibilities/simulators.py	81.81%	2 Missing and 2 partials ⚠️
hera_sim/visibilities/matvis.py	87.50%	1 Missing and 2 partials ⚠️
hera_sim/sigchain.py	89.47%	0 Missing and 2 partials ⚠️
hera_sim/visibilities/cli.py	66.66%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #332      +/-   ##
==========================================
+ Coverage   93.37%   93.42%   +0.05%     
==========================================
  Files          25       25              
  Lines        3428     3333      -95     
  Branches      577      553      -24     
==========================================
- Hits         3201     3114      -87     
+ Misses        122      116       -6     
+ Partials      105      103       -2     

Flag	Coverage Δ
unittests	93.42% <94.08%> (+0.08%)	⬆️

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.

[tyler-a-cox]

Thanks for all the work on this, @steven-murray! I'm mostly okay with the changes here. I was wondering why the PerturbedPolyBeam in the polybeam_simulation notebook has changed so significantly despite using the same coefficients? I'm less familiar with the PolyBeam class, so if these changes are expected, this comment can probably be ignored.

[bhazelton]

I don't know much about the PolyBeam, but looking at the plots, the old beam plot looks to me like a power beam not an efield beam (even though that's what it's called in the notebook). The thing I'm picking up on is that it doesn't seem to have any polarized structure, so I guess the more appropriate thing to say is that it looks like an unpolarized beam. The new plot looks much more like a polarized Efield beam. Looking at the memo on the polybeam, I see no plots that really look like a polarized E-field beam, they all look unpolarized.

And digging into the code, I think I see the difference: in the new code the polarized parameter, which was defaulted to False has been removed and it appears that it is always polarized (the code for the polarized type beam is always run). I assume this was a conscious decision, but it's interesting that there's not an option to get the unpolarized beam which was the default in the past.

I also take slight issue with the way the beam is turned into a polarized beam -- just multiplying by a dipole response. The result looks nothing like the Fagnioni simulations, which are of course polarized.

[steven-murray]

@bhazelton yes you're exactly right -- the difference is due to me removing the unpolarized branch of code. I think I was equating "unpolarized" with "power" beam, and since I only wrote the efield beam evaluation, I removed the unpolarized option. I'm not the original author of the polybeam code, so I'm not totally sure I understand the intention behind having the polarized keyword here. Maybe @philbull can enlighten us further?

[steven-murray]

@bhazelton and @tyler-a-cox I updated the PolyBeam to again take the polarized keyword (with default of False, like in previous versions), which restores the match. We can remove this at a later date as we see fit (I'll make an issue about it). Otherwise, everything should be running through fine, so let's get this merged!

[tyler-a-cox]

Thanks for the PolyBeam fix @steven-murray, this looks good to me!