Multi progbar backend #358
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ 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 ☔ View full report in Codecov by Sentry. |
pyuvsim/uvsim.py
Outdated
| if tag == mpi.tags.READY: | ||
| pass | ||
| elif tag == mpi.tags.DONE: | ||
| uv_container.data_array[msg[1]] += msg[0] | ||
| pbar.update(1) | ||
|
|
||
| elif tag == mpi.tags.EXIT: | ||
| completed_workers += 1 | ||
| else: | ||
| raise ValueError(f"{msg} {tag}") |
There was a problem hiding this comment.
There's two lines in here that just won't ever get touched by tests. the READY tag is never sent back from a worker thread, and I think the else catch was a debug thing that just never went away just in case. Ideally the "enum" defined in the mpi module is an exhaustive list of statuses that can be sent back. Would it be preferred to just take out these lines?
|
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. |
|
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. |
mkolopanis
force-pushed
the
multi_progbar_backend
branch
from
8357105 to
de5680a
Compare
mkolopanis
force-pushed
the
multi_progbar_backend
branch
from
3d49b7f to
c5ee33d
Compare
mkolopanis
force-pushed
the
multi_progbar_backend
branch
from
3f8888d to
08f25e1
Compare
mkolopanis
force-pushed
the
multi_progbar_backend
branch
2 times, most recently
from
7daa109 to
186d4ba
Compare
mkolopanis
force-pushed
the
multi_progbar_backend
branch
2 times, most recently
from
47c6ed9 to
34767ae
Compare
mkolopanis
force-pushed
the
multi_progbar_backend
branch
2 times, most recently
from
bf28b61 to
f87a714
Compare
move inner progsteps to own function add in tqdm progressbar, check diffuse analytic, check for npus > 1 add tqdm to 'all' dependency, try to test importerror in min deps added a two helper functions for chunking iterators added the send_recv backend which combines with tqdm to distribute tasks propagate the backend keyword up to the run script allow all backends to use the profiling option make analytic diffuse test also run against all backends import from pyuvsim.uvsim in the import tqdm error test add tqdm to full test suite, make tests that run with only progsteps add test to run profiling for all backends. only make profiling output if uvdata_indices is not empty remove debug prints, remove thread_multiple as a parameter to run fix number of arguments to run_uvsim test define a proper enum for the tags added tqdm to option list in readme fixed typo in run_param_pyuvsim argument parser force ntasks tot to be an int, ceil to be conservative add a context manger to progsteps disentangle the progressbar from the backend; test all combinations revamp rma to not use rank 0, remove messaging equiv to s/r update tests to run in paralell. do gymnastics for warnings have newer tests account for parallel and backend differences propagate errors on worker nodes to rank0 in send_recv. always clean up the window in rma process. [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci restore mpich test yaml lost to rebase overlords update progsteps only ci yaml match versioning on progsteps_only yaml to other ci yamls break some check from run_uvdata_uvsim into a separate function
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
mkolopanis
force-pushed
the
multi_progbar_backend
branch
from
0d2f1f3 to
984d83d
Compare
for more information, see https://pre-commit.ci
Adds
tqdmprogress bar as a backend option forrun_uvdata_uvsimalso 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: