RadioAstronomySoftwareGroup / pyuvsim

A ultra-high precision package for simulating radio interferometers in python on compute clusters.
https://pyuvsim.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
43 stars 7 forks source link

Add handling and checking for x_orientation #363

Closed bhazelton closed 2 years ago

bhazelton commented 2 years ago

Description

Add a check on the x_orientation of the beams. If it is set consistently on all the beams, use it to set the output uvdata x_orientation. If it is inconsistent across the beams raise an error.

I also moved a function so that it was defined before it was called and I had to do a little refactoring to keep the complexity of create_mock_catalog under the limit.

Motivation and Context

Types of changes

Checklist:

For all pull requests:

Bug fix checklist:

codecov[bot] commented 2 years ago

Codecov Report

Merging #363 (af4f66d) into main (d5a8cf1) will increase coverage by 0.01%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #363      +/-   ##
==========================================
+ Coverage   99.07%   99.08%   +0.01%     
==========================================
  Files          13       13              
  Lines        1941     1963      +22     
==========================================
+ Hits         1923     1945      +22     
  Misses         18       18              
Impacted Files Coverage Δ
pyuvsim/simsetup.py 99.88% <100.00%> (+<0.01%) :arrow_up:
pyuvsim/telescope.py 100.00% <100.00%> (ø)

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 d5a8cf1...af4f66d. Read the comment docs.

bhazelton commented 2 years ago

@steven-murray the hera_sim tests appear to be failing on this PR but I'm not sure it's caused by the changes in this PR. Can you investigate?

bhazelton commented 2 years ago

@steven-murray this PR is failing on the hera_sim tests because of a UVData select on polarization that is asking for the "ee" polarization but it looks like the UVData object doesn't have x_orientation set. I don't really see why this should have been affected by this PR, can you take a look?

steven-murray commented 2 years ago

Ah. I'll look into it more, but I think this is something that will be dealt with properly in the upcoming simulator overhaul for hera_sim. At the moment, polarization for VisCPU is just broken on the main branch.

bhazelton commented 2 years ago

@aelanman can you re-approve this so I can merge it?