Histogram data download #1782

meyerjo · 2019-01-28T10:05:43Z

Motivation for features / changes

This pull requests add the option to download the raw histogram data as described in #1275

Technical description of changes

Add the checkbox "Show data download links" to the histogram page.
Add the download dialog to the individual histogram cards
Change the Query for the histograms in order to allow filtering for the individual experiment

Screenshots of UI changes

The button to trigger the download buttons:

The download selection for the data:

Detailed steps to verify changes work correctly (as executed by you)

Open a tensorboard with histogram data
Go to the histograms page
Select "Show data download links"
Select experiment in the histograms card
Download CSV or JSON

Alternate designs / implementations considered

Regarding the CSV Export: So far, all the BinStart, BinEnd, BinValue fields are a consecutive string which in itself is ';' separated. Thus, one has 5 comma separated columns. One could also just comma separate these values, but this could lead to an inconsistent number of columns.

meyerjo · 2019-01-28T10:30:40Z

Changed the alignment of the download bar

meyerjo · 2019-01-30T15:05:56Z

Don't really see the reason for build failing. The error says pandas is missing. However, this PR did not remove or add any pandas import.

nfelt

Thanks for the contribution!

tensorboard/plugins/histogram/histograms_plugin.py

nfelt · 2019-01-31T04:02:21Z

tensorboard/plugins/histogram/histograms_plugin.py


  def get_plugin_apps(self):
    return {
-        '/histograms': self.histograms_route,


Please revert the whitespace changes to indentation - continuation lines should be indented +4 spaces

tensorboard/plugins/histogram/histograms_plugin.py

nfelt · 2019-01-31T04:09:11Z

tensorboard/plugins/histogram/histograms_plugin.py

+class OutputFormat(object):
+  """An enum used to list the valid output formats for API calls."""
+  JSON = 'json'
+  CSV = 'csv'


Please add a test for the CSV output format. See e.g. the equivalent test for the scalars plugin:

tensorboard/tensorboard/plugins/scalar/scalars_plugin_test.py

Lines 176 to 191 in 1451220

def _test_scalars_csv(self, run_name, tag_name, should_work=True):

self.set_up_with_runs([self._RUN_WITH_LEGACY_SCALARS,

self._RUN_WITH_SCALARS,

self._RUN_WITH_HISTOGRAM])

if should_work:

(data, mime_type) = self.plugin.scalars_impl(

tag_name, run_name, None, scalars_plugin.OutputFormat.CSV)

self.assertEqual('text/csv', mime_type)

s = StringIO(data)

reader = csv.reader(s)

self.assertEqual(['Wall time', 'Step', 'Value'], next(reader))

self.assertEqual(len(list(reader)), self._STEPS)

else:

with self.assertRaises(KeyError):

self.plugin.scalars_impl(self._SCALAR_TAG, run_name, None,

scalars_plugin.OutputFormat.CSV)

I added a test case. Is there a way to trigger the tests on my local machine? Didn't find anything in DEVELOPMENT.md oder CONTRIBUTING.md

You can run bazel test --test_output=errors //tensorboard/... to run all tests, or bazel test --test_output=errors //tensorboard/plugins/histogram:histogram_plugin_test to run just the one you changed.

tensorboard/plugins/histogram/tf_histogram_dashboard/tf-histogram-loader.html

tensorboard/plugins/histogram/histograms_plugin.py

* indention changes

nfelt · 2019-02-01T22:41:18Z

tensorboard/plugins/histogram/histograms_plugin.py

+class OutputFormat(object):
+  """An enum used to list the valid output formats for API calls."""
+  JSON = 'json'
+  CSV = 'csv'


You can run bazel test --test_output=errors //tensorboard/... to run all tests, or bazel test --test_output=errors //tensorboard/plugins/histogram:histogram_plugin_test to run just the one you changed.

tensorboard/plugins/histogram/histograms_plugin.py

tensorboard/plugins/scalar/scalars_plugin.py

tensorboard/plugins/histogram/histograms_plugin.py

* change the csv writer and directly write the csv

wchargin

It looks like spurious whitespace changes keep being reintroduced. We
don’t want to merge these because they mess up the blame, making it
harder to tell when a part of a file was most recently substantively
changed. Please revert these changes. If your editor is automatically
formatting files, you might want to configure it to not do that.

Once the tests pass and the diff is clean, please post a comment here to
request a re-review. Thanks!

wchargin · 2019-02-14T18:51:30Z

tensorboard/plugins/histogram/tf_histogram_dashboard/tf-histogram-loader.html

        icon="fullscreen"
        on-tap="_toggleExpanded"
      ></paper-icon-button>
+      <template is="dom-if" if="[[showDownloadLinks]]">


Rather than reimplementing this, please use the tf-downloader element.
You can see an example of its use in tf-scalar-card as of #1846.

wchargin · 2019-02-14T18:54:48Z

tensorboard/plugins/histogram/histograms_plugin.py

+        [
+          e[0],
+          e[1],
+          ';'.join(['{:.17f}'.format(el[0]) for el in e[2]]),


Why format with fixed precision as opposed to just str(el[0])?

wchargin · 2019-02-14T18:57:52Z

tensorboard/plugins/histogram/histograms_plugin.py

+      events: A list of (wall_time, step, tensor_as_list) event tuples.
+
+    Returns:
+      CSV string representation of the given events.


Please document the representation. There’s no reasonable way for
someone to correctly guess the form of CSV returned by this handler
without reading its implementation. By reading this documentation (and
nothing more), I should be able to confidently write code that processes
the CSV returned by this function.

wchargin · 2019-02-14T19:11:19Z

tensorboard/plugins/histogram/histograms_plugin_test.py

+                              self._LEGACY_HISTOGRAM_TAG)
+
+  def test_histograms_csv_with_histogram(self):
+    self._test_histograms_csv(self._RUN_WITH_HISTOGRAM, self._HISTOGRAM_TAG,


self._test_histograms_csv(self._RUN_WITH_HISTOGRAM, self._HISTOGRAM_TAG, should_work=False)

This indeed fails, but not for the right reason. A histogram run with a
histogram tag should work, but you’ve specified the wrong tag: it
should be "%s/histogram_summary" % self._HISTOGRAM_TAG, as listed in
the test_histograms_with_histogram test case above.

Please add a test case that actually should fail (e.g., requesting a
scalar tag on a histograms run).

wchargin · 2019-02-14T19:11:39Z

tensorboard/plugins/histogram/tf_histogram_dashboard/tf-histogram-loader.html

-                router.pluginRoute('histograms', '/histograms'),
-                {tag, run});
+              router.pluginRoute('histograms', '/histograms'),
+              {


Please revert this reformatting change.

wchargin · 2019-02-14T19:11:53Z

tensorboard/plugins/scalar/scalars_plugin.py

    experiment = request.args.get('experiment')
    output_format = request.args.get('format')
    (body, mime_type) = self.scalars_impl(tag, run, experiment, output_format)
+


Please revert this newline.

meyerjo added 4 commits January 25, 2019 17:34

* removed TODO and return valid http response in case of an error

7441f07

* adding ability to download the histogram raw data from tensorboard

7677de4

* code style

9f64ad2

* fixed an alignment error in the histogram error

882755b

* removed freference to scalars_plugin

1451220

nfelt suggested changes Jan 31, 2019

View reviewed changes

meyerjo added 5 commits January 31, 2019 11:41

* some of the requested changes

6c150bf

* added a helper function for the csv response

b0f7d85

* removed the experiment id from the sql statement

7fad678

* indention changes

* add testcase for csv export

c184274

* changed import

07b200f

nfelt reviewed Feb 1, 2019

View reviewed changes

meyerjo added 11 commits February 11, 2019 14:46

* change csv writer comment style

421c32e

* change the csv writer and directly write the csv

* changed order of arguments

f8a91d8

* revert whitespace changes

25b2bcc

* change test files

9ce41a6

* using named parameters

aa42a34

* revert change to scalars_plugin

a44e96a

* whitespace changes

b584cb4

* changed test cases

3033069

* ordering imports

b618bb4

* worked on tests

18bc96c

* changed histogram test

665110d

wchargin reviewed Feb 14, 2019

View reviewed changes

	def _test_scalars_csv(self, run_name, tag_name, should_work=True):
	self.set_up_with_runs([self._RUN_WITH_LEGACY_SCALARS,
	self._RUN_WITH_SCALARS,
	self._RUN_WITH_HISTOGRAM])
	if should_work:
	(data, mime_type) = self.plugin.scalars_impl(
	tag_name, run_name, None, scalars_plugin.OutputFormat.CSV)
	self.assertEqual('text/csv', mime_type)
	s = StringIO(data)
	reader = csv.reader(s)
	self.assertEqual(['Wall time', 'Step', 'Value'], next(reader))
	self.assertEqual(len(list(reader)), self._STEPS)
	else:
	with self.assertRaises(KeyError):
	self.plugin.scalars_impl(self._SCALAR_TAG, run_name, None,
	scalars_plugin.OutputFormat.CSV)

Histogram data download #1782

Are you sure you want to change the base?

Histogram data download #1782

Uh oh!

Conversation

meyerjo commented Jan 28, 2019

Uh oh!

meyerjo commented Jan 28, 2019

Uh oh!

meyerjo commented Jan 30, 2019

Uh oh!

nfelt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nfelt Feb 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nfelt Feb 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wchargin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nfelt Feb 1, 2019 •

edited

Loading

nfelt Feb 1, 2019 •

edited

Loading