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

Code Review of simsetup #413

Open steven-murray opened 1 year ago

steven-murray commented 1 year ago

This issue is mostly a reminder that we should do a bit of a group review of the simsetup part of pyuvsim. A few notes:

  1. There seem to be two main parts of pyuvsim: (i) the configuration definition (i.e. simsetup) and (ii) the actual simulator, which is supposed to provide a community 'reference'. To me, it seems that part (i) is essential because it allows other simulators to be run based on the same configuration file as pyuvsim, and therefore more easily compared.
  2. The simsetup module is quite convoluted at this point (a few open issues testify to this), and could do with a reduction of complexity.
  3. The simsetup module was written without too much of a concern for performance, because it was assumed that the simulation part would always be the biggest bottleneck. However, this is not always the case, depending on how the simulator itself works (eg. in HERA validation using vis-cpu, we do the simsetup at least once for each frequency, sometimes multiple times for each frequency). This brings the 'overhead' from simsetup into a regime where it can be very non-negligible (see eg. #410).

In terms of performance, I think ideas would revolve around how to streamline UVData creation, and which (if any) checks need to be run on the uvdata object, and which attributes "need" to be set.

In terms of code complexity, I wonder if using pyyaml's in-built ability to define tags etc might not come in handy (maybe not, but it's something to consider).