-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
[data] Only dump state once on progress bar close #46928
[data] Only dump state once on progress bar close #46928
Conversation
Signed-off-by: Matthew Owen <[email protected]>
54aafe0
to
e88bd35
Compare
Signed-off-by: Matthew Owen <[email protected]>
Signed-off-by: Matthew Owen <[email protected]>
@@ -110,6 +110,11 @@ def execute( | |||
|
|||
logger.debug("Execution config: %s", self._options) | |||
|
|||
# Note: DAG must be initialized in order to query num_outputs_total. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you add a comment indicating why this must happen prior to operator bars being initialized, so readers know about this fix?
@@ -241,8 +241,10 @@ def update_bar(self, state: ProgressBarState) -> None: | |||
def close_bar(self, state: ProgressBarState) -> None: | |||
"""Remove a bar from this group.""" | |||
bar = self.bars_by_uuid[state["uuid"]] | |||
instance().hide_bars() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, let's explain why we are hiding/unhiding the bar
python/ray/experimental/tqdm_ray.py
Outdated
# Don't bother if ray is shutdown (in __del__ hook). | ||
self._closed = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we can put this in the original order to minimize change
Signed-off-by: Matthew Owen <[email protected]>
Signed-off-by: Matthew Owen <[email protected]>
Signed-off-by: Matthew Owen <[email protected]>
Close ray-project#44979. This also addresses an issue where the bars were misordered in jupyter notebooks (the overall dataset bar showed up last). --------- Signed-off-by: Matthew Owen <[email protected]> Signed-off-by: Dev <[email protected]>
Why are these changes needed?
Close #44979. This also addresses an issue where the bars were misordered in jupyter notebooks (the overall dataset bar showed up last). Examples after the change are below:
Basic Example
Jupyter Notebook
basic_ipynb.mov
Terminal
basic_term.mov
Many Operators Example
Jupyter Notebook
many_bars_ipynb.mov
Terminal
many_bars_term.mov
Many Datasets Example
Jupyter Notebook
many_datasets_ipynb.mov
Terminal
many_datasets_term.mov
Related issue number
Close #44979
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.