Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code Review of simsetup #413

Open
steven-murray opened this issue Dec 5, 2022 · 0 comments
Open

Code Review of simsetup #413

steven-murray opened this issue Dec 5, 2022 · 0 comments

Comments

@steven-murray
Copy link
Contributor

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. Sim Setup Performance Enhancements #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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants