uploader: display upload statistics while uploading data #3678

caisq · 2020-05-28T20:23:42Z

Motivation for features / changes
- This is the first of a series of PRs aimed at improving the usability of the tensorboard.dev uploader.
- This PR specifically adds real-time display of upload statistics.
Technical description of changes
- During uploading, the users sees 2 or 3 bars rendered with tqdm:
  1. [TIMESTAMP] Uploaded x scalars, y tensors (Y B), z binary objects (Z B)
  2. (if any data was skipped): Skipped n tensors (N B), m binary objects (M B)
  3. Uploading XYZ (this is the real-time status)
- If the user wishes to see a less verbose UI, they can use --verbose 0 to override
  the default value 1
- When the uploading is idling, it displays "Listening for new data in logdir..."
- Add a module upload_tracker.py to encapsulate the logic for stats tracking
  and display.
Screenshots of UI changes
- While uploading actively:
- While idling:
Detailed steps to verify changes work correctly (as executed by you)
- Unit tests added for the new module and its usage inside the existing uploader.py module
- Manual verification (see screenshots above)

caisq · 2020-05-28T20:36:28Z

davidsoergel · 2020-06-02T14:49:53Z

tensorboard/uploader/uploader.py

@@ -503,7 +534,12 @@ def flush(self):

        self._rpc_rate_limiter.tick()

-        with _request_logger(request, request.runs):
+        with contextlib.ExitStack() as stack:


Took me a bit to figure out why you did it this way :) I think it's because self._tracker might be None, so you can't just with self._tracker.scalars_tracker(...):.

I think this would be clearer, more idiomatic, and more flexible if you don't allow self._tracker to be None, but instead push the verbosity argument down into UploadTracker and handle it there (i.e., checking the condition only when it comes time to actually print something). Then all of these ExitStack things go away and you can just say

with _request_logger(request, request.runs): with self._tracker.scalars_tracker(self._num_values): ...

Done. This increases the maximum depth of ident by 1. But I think it's worth the benefits you pointed out.

davidsoergel · 2020-06-02T14:51:06Z

tensorboard/uploader/uploader.py

@@ -157,6 +163,9 @@ def create_experiment(self):
        response = grpc_util.call_with_retries(
            self._api.CreateExperiment, request
        )
+        self._tracker = (


Per comment below, how about

self._tracker = upload_tracker.UploadTracker(self._verbosity)

(and all the consequences of that refactoring)

caisq

Thanks for the review!

caisq · 2020-06-02T23:51:56Z

tensorboard/uploader/uploader.py

@@ -503,7 +534,12 @@ def flush(self):

        self._rpc_rate_limiter.tick()

-        with _request_logger(request, request.runs):
+        with contextlib.ExitStack() as stack:


Done. This increases the maximum depth of ident by 1. But I think it's worth the benefits you pointed out.

caisq · 2020-06-02T23:52:11Z

tensorboard/uploader/uploader.py

@@ -157,6 +163,9 @@ def create_experiment(self):
        response = grpc_util.call_with_retries(
            self._api.CreateExperiment, request
        )
+        self._tracker = (


davidsoergel

Nice! Very thorough tests, too :)

davidsoergel · 2020-06-04T14:45:29Z

tensorboard/uploader/uploader.py

+                    sent_blobs += self._send_blob(
+                        blob_sequence_id, seq_index, blob
+                    )
+                    if self._tracker:


this condition is no longer needed

davidsoergel · 2020-06-04T14:46:22Z

tensorboard/uploader/uploader.py

@@ -594,6 +622,7 @@ def __init__(
        rpc_rate_limiter,
        max_request_size,
        max_tensor_point_size,
+        tracker=None,


Don't default to None, now that this is required

Done. Same for another line like this.

davidsoergel · 2020-06-04T14:46:56Z

tensorboard/uploader/uploader.py

+        api,
+        rpc_rate_limiter,
+        max_request_size,
+        tracker=None,


Don't default to None, now that this is required

caisq

Thanks for the review!

caisq · 2020-06-04T15:20:59Z

tensorboard/uploader/uploader.py

+        api,
+        rpc_rate_limiter,
+        max_request_size,
+        tracker=None,


caisq · 2020-06-04T15:21:13Z

tensorboard/uploader/uploader.py

@@ -594,6 +622,7 @@ def __init__(
        rpc_rate_limiter,
        max_request_size,
        max_tensor_point_size,
+        tracker=None,


Done. Same for another line like this.

caisq · 2020-06-04T19:08:32Z

I've removed the usage of tqdm, because of two reasons

The multi-progress-bar setup used previously in this PR doesn't work well in notebooks, as I discovered through manual testing. So I've reverted to a sys.stdout writing and flushing only approach.
We're currently not using the main progress-bar feature of tqdm. So it seems to be prudent to avoid adding this new dependency for now.

uploader: display upload statistics while uploading data

d3b4446

googlebot added the cla: yes label May 28, 2020

caisq added 2 commits May 28, 2020 16:32

Fixes

69a7c4e

Fix license year; Make some vars private

a3dc1c7

caisq requested a review from davidsoergel May 28, 2020 20:36

s/pandas/tqdm

478d368

davidsoergel reviewed Jun 2, 2020

View reviewed changes

Address comments

50046b2

caisq commented Jun 2, 2020

View reviewed changes

Fix comment

2a97c81

caisq requested a review from davidsoergel June 2, 2020 23:54

davidsoergel approved these changes Jun 4, 2020

View reviewed changes

Address comments

ffd56e2

caisq commented Jun 4, 2020

View reviewed changes

caisq added 2 commits June 4, 2020 11:57

Fix py lint

76c37cc

Remove usage of tqdm

d7b82df

caisq added 4 commits June 4, 2020 15:09

Remove tqdm from requirements.txt

ddee63c

Remove cruft

2fccb04

Merge branch 'master' into uploader-status-1

7b7491d

Wording tweak

5f6a313

caisq merged commit 67801ef into tensorflow:master Jun 4, 2020

caisq mentioned this pull request Jun 5, 2020

uploader: throttle text output #3711

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

uploader: display upload statistics while uploading data #3678

uploader: display upload statistics while uploading data #3678

caisq commented May 28, 2020 •

edited

Loading

caisq commented May 28, 2020

davidsoergel Jun 2, 2020

caisq Jun 2, 2020

davidsoergel Jun 2, 2020

caisq Jun 2, 2020

caisq left a comment

caisq Jun 2, 2020

caisq Jun 2, 2020

davidsoergel left a comment

davidsoergel Jun 4, 2020

caisq Jun 4, 2020

davidsoergel Jun 4, 2020

caisq Jun 4, 2020

davidsoergel Jun 4, 2020

caisq Jun 4, 2020

caisq left a comment

caisq Jun 4, 2020

caisq Jun 4, 2020

caisq commented Jun 4, 2020

uploader: display upload statistics while uploading data #3678

uploader: display upload statistics while uploading data #3678

Conversation

caisq commented May 28, 2020 • edited Loading

caisq commented May 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caisq left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidsoergel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caisq left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caisq commented Jun 4, 2020

caisq commented May 28, 2020 •

edited

Loading