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

Multi progbar backend #358

Open mkolopanis opened 3 years ago

mkolopanis commented 3 years ago

Adds tqdm progress bar as a backend option for run_uvdata_uvsim also implements a send and receive protocol for run_uvdata_uvsim where the tasks are chunked and sent out to PUs on request.

Motivation and Context

I like how tqdm looks honestly and was tired of two gross branches continually rebasing. This allows for all backends depending on the user's preference. I think I would prefer having some kind of plugin or something to allow custom progress bars but that sounds much more complicated.

TODO:

Types of changes

Checklist:

For all pull requests:

New feature checklist:

codecov[bot] commented 3 years ago

Codecov Report

Attention: Patch coverage is 96.33028% with 8 lines in your changes missing coverage. Please review.

Project coverage is 98.44%. Comparing base (0876a90) to head (f4e5b25).

Files Patch % Lines
src/pyuvsim/uvsim.py 95.72% 8 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #358 +/- ## =========================================== - Coverage 100.00% 98.44% -1.56% =========================================== Files 11 11 Lines 2165 2317 +152 =========================================== + Hits 2165 2281 +116 - Misses 0 36 +36 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

aelanman commented 3 years ago

While I fully support switching to tqdm over progsteps, I'm still not convinced of the push/pull model for task distribution. Has it been shown conclusively that active distribution performs better than the existing system? It looks like it should be capable of about the same performance except with an added overhead from using blocking sends to return visibilities to rank 0.

mkolopanis commented 3 years ago

Definitely part of this endeavor is to just get the technology in the repo right alongside the other things in order to test it better. There were branches before, but it was almost impossible to rebase or something after a new feature was brought in. Even just sitting on this branch, it is much easier to make a comparison.

I think one thing we should do is to get a few really good timing tests. We could always drop the send/recv part in the end.