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 pyradiosky version to output file #384

Closed bhazelton closed 2 years ago

bhazelton commented 2 years ago

Description

Note: In order to have all the required info in run_uvdata_uvsim for the history, I needed to add some attributes to SkyModelData. I also needed to add the input catalog filename to the SkyModel object created in simsetup.initialize_catalog_from_params. For the current and older versions of pyradiosky, this will be a simple attribute. But I also made a PR on pyradiosky to add a filename attribute more generally (see https://github.com/RadioAstronomySoftwareGroup/pyradiosky/pull/169). The implementation in this PR works with both the current pyradiosky and with the branch in that PR.

Motivation and Context

fixes #327

Types of changes

Checklist:

For all pull requests:

New feature checklist:

codecov[bot] commented 2 years ago

Codecov Report

Merging #384 (8ea53d2) into main (1f88b96) will increase coverage by 0.15%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #384      +/-   ##
==========================================
+ Coverage   99.17%   99.32%   +0.15%     
==========================================
  Files          13       13              
  Lines        2054     2068      +14     
==========================================
+ Hits         2037     2054      +17     
+ Misses         17       14       -3     
Impacted Files Coverage Δ
pyuvsim/simsetup.py 99.88% <100.00%> (+<0.01%) :arrow_up:
pyuvsim/uvsim.py 99.42% <100.00%> (+0.86%) :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 1f88b96...8ea53d2. Read the comment docs.

bhazelton commented 2 years ago

@steven-murray We added this to the same place where we add the pyuvsim and pyuvdata version info in run_uvsim, which is the top-level function that is generally called to run simulations. However we note that hera_sim doesn't call that function, it calls run_uvdata_uvsim, a lower-level function. Is there a good reason to call that instead of run_uvsim? If so, we should probably ensure that the version numbers get into output files if you run that function as well.

steven-murray commented 2 years ago

@bhazelton the reason we use run_uvdata_uvsim is essentially to maintain API parity between different simulators, which is the whole point of the hera_sim.visibilities module. The idea is to use pyuvsim to read config files, then use the UVData/UVBeam/SkyModel classes created from those configs to run a simulator -- not necessarily pyuvsim.

Most other simulators don't have a function that will directly run from the config file, which makes it necessary for hera_sim to provide this layer. Note that hera_sim also supports running directly from objects, rather than config files, which is another reason this is necessary.

bhazelton commented 2 years ago

Ok, then I think we need to move the history modifications so that they get picked up by both functions.