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

Fix code to use new pyradiosky and pyuvdata methods #375

Closed bhazelton closed 2 years ago

bhazelton commented 2 years ago

Description

Motivation and Context

Stay up to date with changes in pyuvdata & pyradiosky:

Types of changes

Checklist:

For all pull requests:

Bug fix checklist:

New feature checklist:

codecov[bot] commented 2 years ago

Codecov Report

Merging #375 (65bddc5) into main (d644607) will increase coverage by 0.05%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #375      +/-   ##
==========================================
+ Coverage   99.09%   99.14%   +0.05%     
==========================================
  Files          13       13              
  Lines        1980     1997      +17     
==========================================
+ Hits         1962     1980      +18     
+ Misses         18       17       -1     
Impacted Files Coverage Δ
pyuvsim/antenna.py 96.07% <100.00%> (+1.84%) :arrow_up:
pyuvsim/simsetup.py 99.88% <100.00%> (+<0.01%) :arrow_up:
pyuvsim/utils.py 98.10% <100.00%> (+0.12%) :arrow_up:
pyuvsim/uvsim.py 98.55% <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 d644607...65bddc5. Read the comment docs.

bhazelton commented 2 years ago

There are some checkmarks in the PR template about reproducing the reference sims. I haven't done that yet, are there scripts for that?

mkolopanis commented 2 years ago

I think we decided that it should not be necessary for complete reproduction of the reference sims for this PR. Maybe wouldn't hurt to run sim 2.1 though on a cluster? That one takes a few hours generally.

bhazelton commented 2 years ago

I think that would be more than sufficient and if you're willing, I'd be grateful. I was going to try to figure out how to run the v1 sims on my own machine but haven't had the time yet.

mkolopanis commented 2 years ago

Yeah I'll run them and report back.

mkolopanis commented 2 years ago

Reference sim 2.1 took 516.642 min, on par with the ~8.5ish hours we saw in the PR we just merged. I didn't think a comparison with main was strictly necessary since this PR has such quick turnaround with the blt ordering one.

bhazelton commented 2 years ago

@mkolopanis did you also check that it got the same results? If so, I think this might be ready for another review.

mkolopanis commented 2 years ago

So this is a fun point. We don't have a saved reference sim 2.1 on the google drive. I can check that it matches some other reference sim 2.1 outputs.

mkolopanis commented 2 years ago

But yes all the results match the previous simulations I have run for timings (including from the old main branch). in that case. I agree ready for a review

bhazelton commented 2 years ago

@aelanman I think this PR is ready if you want to have another look.