-
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] Simplify and consolidate progress bar outputs #47692
Changes from 1 commit
4db8321
bc918f3
3e3b70d
12d4455
1969bac
af218fc
11ed8f5
8f9873d
7861e8d
66f5692
5ef5aa8
75a0cd2
00c43b0
1c892da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -259,16 +259,16 @@ def refresh_progress_bar(self, resource_manager: ResourceManager) -> None: | |
def summary_str(self, resource_manager: ResourceManager) -> str: | ||
queued = self.num_queued() + self.op.internal_queue_size() | ||
active = self.op.num_active_tasks() | ||
desc = f"- {self.op.name}: {active} active, {queued} queued" | ||
desc = f"- {self.op.name}. Tasks: {active} 🟢, {queued} 🟡" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "N queued" actually means N blocks in the input buffer, not number of tasks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also since we have a "Tasks: " section here. I'm wondering maybe we can also move the actor info after this. Instead of at the very end. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe format it as "Tasks ..., Actors ..., Input blocks ..." There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also, and blocked sign can be part of "tasks" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
+1 something like this seems reasonable to me There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do the green and yellow circles represent? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. green = active, yellow = queued There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. although after reworking the progress bar as suggested above, i have removed the green/yellow emoji |
||
if ( | ||
self.op._in_task_submission_backpressure | ||
or self.op._in_task_output_backpressure | ||
): | ||
desc += " 🚧" | ||
desc += f", [{resource_manager.get_op_usage_str(self.op)}]" | ||
desc += f"; Resource usage: {resource_manager.get_op_usage_str(self.op)}" | ||
suffix = self.op.progress_str() | ||
if suffix: | ||
desc += f", {suffix}" | ||
desc += f"; {suffix}" | ||
return desc | ||
|
||
def dispatch_next_task(self) -> None: | ||
|
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.
can we hide the pending section if all are 0? (forgot to mention in the previous PR)