From c2bf6b64416f1373cc5835218125540714b4fac9 Mon Sep 17 00:00:00 2001 From: William Chargin Date: Mon, 19 Aug 2019 16:02:30 -0700 Subject: [PATCH 1/3] data: plumb experiment ID through runs and scalars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: This implements a rudimentary experiment selection mechanism for the runs selector and scalars dashboard only, for the purposes of testing data provider implementations. The experiment ID is stored as a query parameter, which isn’t perfect because it precludes caching the big TensorBoard HTML blob across experiments. In the long term, we can consider refactoring to serve the big HTML blob from a static path that’s fetched by the page. The TensorBoard codebase still has some fragments of a previous experimental data selector, which was partially removed in #2290, and so parts of this diff look like changes but are functionally additions. Test Plan: Instrument the multiplexer data provider to log experiment IDs: ```diff diff --git a/tensorboard/backend/event_processing/data_provider.py b/tensorboard/backend/event_processing/data_provider.py index ef602320..4c72d096 100644 --- a/tensorboard/backend/event_processing/data_provider.py +++ b/tensorboard/backend/event_processing/data_provider.py @@ -54,7 +54,7 @@ class MultiplexerDataProvider(provider.DataProvider): return None def list_runs(self, experiment_id): - del experiment_id # ignored for now + logger.warn("Listing runs for experiment %r", experiment_id) return [ provider.Run( run_id=run, # use names as IDs @@ -65,7 +65,7 @@ class MultiplexerDataProvider(provider.DataProvider): ] def list_scalars(self, experiment_id, plugin_name, run_tag_filter=None): - del experiment_id # ignored for now + logger.warn("Listing scalars for experiment %r", experiment_id) run_tag_content = self._multiplexer.PluginRunToTagToContent(plugin_name) result = {} if run_tag_filter is None: @@ -96,6 +96,7 @@ class MultiplexerDataProvider(provider.DataProvider): def read_scalars( self, experiment_id, plugin_name, downsample=None, run_tag_filter=None ): + logger.warn("Reading scalars for experiment %r", experiment_id) # TODO(@wchargin): Downsampling not implemented, as the multiplexer # is already downsampled. We could downsample on top of the existing # sampling, which would be nice for testing. ``` Then launch TensorBoard with `--generic_data=true` and navigate to ; verify that all three varieties of server logs are exclusively for the correct experiment ID. Then, remove the query parameter, and ensure that TensorBoard still works normally (with experiment ID the empty string). wchargin-branch: data-experiment --- tensorboard/components/tf_backend/router.ts | 8 ++++++-- tensorboard/components/tf_backend/runsStore.ts | 8 +++++++- tensorboard/plugins/core/core_plugin.py | 4 ++-- tensorboard/plugins/scalar/scalars_plugin.py | 11 ++++++----- .../tf_scalar_dashboard/tf-scalar-card.html | 2 +- .../tf-scalar-dashboard.html | 17 +++++++++++++++-- 6 files changed, 37 insertions(+), 13 deletions(-) diff --git a/tensorboard/components/tf_backend/router.ts b/tensorboard/components/tf_backend/router.ts index e9ba029ca9..21fbfc3a9f 100644 --- a/tensorboard/components/tf_backend/router.ts +++ b/tensorboard/components/tf_backend/router.ts @@ -22,7 +22,7 @@ namespace tf_backend { params?: URLSearchParams ) => string; pluginsListing: () => string; - runs: () => string; + runs: (experiment?: string) => string; runsForExperiment: (id: tf_backend.ExperimentId) => string; } @@ -53,7 +53,11 @@ namespace tf_backend { ); }, pluginsListing: () => createDataPath(dataDir, '/plugins_listing'), - runs: () => createDataPath(dataDir, '/runs'), + runs: (experiment?: string) => { + const searchParams = new URLSearchParams(); + searchParams.set('experiment', experiment || ''); + return createDataPath(dataDir, '/runs', searchParams); + }, runsForExperiment: (id) => { return createDataPath( dataDir, diff --git a/tensorboard/components/tf_backend/runsStore.ts b/tensorboard/components/tf_backend/runsStore.ts index b8c870ccb7..5c1554bf97 100644 --- a/tensorboard/components/tf_backend/runsStore.ts +++ b/tensorboard/components/tf_backend/runsStore.ts @@ -17,7 +17,7 @@ namespace tf_backend { private _runs: string[] = []; load() { - const url = getRouter().runs(); + const url = getRouter().runs(this._getExperimentId()); return this.requestManager.request(url).then((newRuns) => { if (!_.isEqual(this._runs, newRuns)) { this._runs = newRuns; @@ -26,6 +26,12 @@ namespace tf_backend { }); } + _getExperimentId() { + return ( + new URLSearchParams(window.location.search).get('experiment') || '' + ); + } + /** * Get the current list of runs. If no data is available, this will be * an empty array (i.e., there is no distinction between "no runs" and diff --git a/tensorboard/plugins/core/core_plugin.py b/tensorboard/plugins/core/core_plugin.py index 8a9e148bfc..ebb76c285a 100644 --- a/tensorboard/plugins/core/core_plugin.py +++ b/tensorboard/plugins/core/core_plugin.py @@ -154,9 +154,9 @@ def _serve_runs(self, request): last, and then ties are broken by sorting on the run name. """ if self._data_provider: + experiment = request.args.get('experiment', '') runs = sorted( - # (`experiment_id=None` as experiment support is not yet implemented) - self._data_provider.list_runs(experiment_id=None), + self._data_provider.list_runs(experiment_id=experiment), key=lambda run: ( run.start_time if run.start_time is not None else float('inf'), run.run_name, diff --git a/tensorboard/plugins/scalar/scalars_plugin.py b/tensorboard/plugins/scalar/scalars_plugin.py index 41c3db3cc1..74e6eebcb1 100644 --- a/tensorboard/plugins/scalar/scalars_plugin.py +++ b/tensorboard/plugins/scalar/scalars_plugin.py @@ -93,11 +93,11 @@ def frontend_metadata(self): element_name='tf-scalar-dashboard', ) - def index_impl(self): + def index_impl(self, experiment=None): """Return {runName: {tagName: {displayName: ..., description: ...}}}.""" if self._data_provider: mapping = self._data_provider.list_scalars( - experiment_id=None, # experiment support not yet implemented + experiment_id=experiment, plugin_name=metadata.PLUGIN_NAME, ) result = {run: {} for run in mapping} @@ -155,7 +155,7 @@ def scalars_impl(self, tag, run, experiment, output_format): # logic. SAMPLE_COUNT = 1000 all_scalars = self._data_provider.read_scalars( - experiment_id=None, # experiment support not yet implemented + experiment_id=experiment, plugin_name=metadata.PLUGIN_NAME, downsample=SAMPLE_COUNT, run_tag_filter=provider.RunTagFilter(runs=[run], tags=[tag]), @@ -224,7 +224,8 @@ def _get_value(self, scalar_data_blob, dtype_enum): @wrappers.Request.application def tags_route(self, request): - index = self.index_impl() + experiment = request.args.get('experiment', '') + index = self.index_impl(experiment=experiment) return http_util.Respond(request, index, 'application/json') @wrappers.Request.application @@ -233,7 +234,7 @@ def scalars_route(self, request): # TODO: return HTTP status code for malformed requests tag = request.args.get('tag') run = request.args.get('run') - experiment = request.args.get('experiment') + experiment = request.args.get('experiment', '') output_format = request.args.get('format') (body, mime_type) = self.scalars_impl(tag, run, experiment, output_format) return http_util.Respond(request, body, mime_type) diff --git a/tensorboard/plugins/scalar/tf_scalar_dashboard/tf-scalar-card.html b/tensorboard/plugins/scalar/tf_scalar_dashboard/tf-scalar-card.html index 54198b3f7b..abc5a648bf 100644 --- a/tensorboard/plugins/scalar/tf_scalar_dashboard/tf-scalar-card.html +++ b/tensorboard/plugins/scalar/tf_scalar_dashboard/tf-scalar-card.html @@ -241,7 +241,7 @@ new URLSearchParams({ tag, run, - experiment: experiment ? experiment.id : '', + experiment: experiment || '', }) ); }; diff --git a/tensorboard/plugins/scalar/tf_scalar_dashboard/tf-scalar-dashboard.html b/tensorboard/plugins/scalar/tf_scalar_dashboard/tf-scalar-dashboard.html index c9d78b3107..415449169e 100644 --- a/tensorboard/plugins/scalar/tf_scalar_dashboard/tf-scalar-dashboard.html +++ b/tensorboard/plugins/scalar/tf_scalar_dashboard/tf-scalar-dashboard.html @@ -309,7 +309,13 @@

No scalar data was found.

}); }, _fetchTags() { - const url = tf_backend.getRouter().pluginRoute('scalars', '/tags'); + const url = tf_backend + .getRouter() + .pluginRoute( + 'scalars', + '/tags', + new URLSearchParams({experiment: this._getExperimentId()}) + ); return this._requestManager.request(url).then((runToTagInfo) => { if (_.isEqual(runToTagInfo, this._runToTagInfo)) { // No need to update anything if there are no changes. @@ -345,15 +351,22 @@

No scalar data was found.

selectedRuns, query ); + const experiment = this._getExperimentId(); categories.forEach((category) => { category.items = category.items.map((item) => ({ tag: item.tag, - series: item.runs.map((run) => ({run, tag: item.tag})), + series: item.runs.map((run) => ({run, tag: item.tag, experiment})), })); }); this.updateArrayProp('_categories', categories, this._getCategoryKey); }, + _getExperimentId() { + return ( + new URLSearchParams(window.location.search).get('experiment') || '' + ); + }, + _tagMetadata(category, runToTagsInfo, item) { const tag = item.tag; const runToTagInfo = {}; From 38f22dddc0dac02768a5ee72d3c7c758fc6b474f Mon Sep 17 00:00:00 2001 From: William Chargin Date: Tue, 20 Aug 2019 16:32:44 -0700 Subject: [PATCH 2/3] [update patch] wchargin-source: 51e9f37203db775b7ca7d0c8b80dc5de44947e32 wchargin-branch: data-experiment --- tensorboard/components/tf_backend/test/backendTests.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tensorboard/components/tf_backend/test/backendTests.ts b/tensorboard/components/tf_backend/test/backendTests.ts index 4225e37540..2e1f2fc272 100644 --- a/tensorboard/components/tf_backend/test/backendTests.ts +++ b/tensorboard/components/tf_backend/test/backendTests.ts @@ -82,18 +82,18 @@ namespace tf_backend { it('leading slash in pathPrefix is an absolute path', () => { const router = createRouter('/data/'); - assert.equal(router.runs(), '/data/runs'); + assert.equal(router.runs(), '/data/runs?experiment='); }); it('returns complete pathname when pathPrefix omits slash', () => { const router = createRouter('data/'); - assert.equal(router.runs(), 'data/runs'); + assert.equal(router.runs(), 'data/runs?experiment='); }); it('does not prune many leading slashes that forms full url', () => { const router = createRouter('///data/hello'); - // This becomes 'http://data/hello/runs' - assert.equal(router.runs(), '///data/hello/runs'); + // This becomes 'http://data/hello/runs?experiment=' + assert.equal(router.runs(), '///data/hello/runs?experiment='); }); it('returns correct value for #environment', () => { @@ -177,7 +177,7 @@ namespace tf_backend { }); it('returns correct value for #runs', () => { - assert.equal(router.runs(), 'data/runs'); + assert.equal(router.runs(), 'data/runs?experiment='); }); it('returns correct value for #runsForExperiment', () => { From 8929ef8f0291ed5ab1e42471aff4b081d708602e Mon Sep 17 00:00:00 2001 From: William Chargin Date: Wed, 21 Aug 2019 17:46:19 -0700 Subject: [PATCH 3/3] [update patch] wchargin-source: 1387baa2ffc19f8733507e836a986fb73b4b7c4e wchargin-branch: data-experiment --- tensorboard/components/tf_backend/router.ts | 7 +++++++ tensorboard/components/tf_backend/runsStore.ts | 8 +------- .../tf_scalar_dashboard/tf-scalar-dashboard.html | 10 ++-------- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/tensorboard/components/tf_backend/router.ts b/tensorboard/components/tf_backend/router.ts index 21fbfc3a9f..77bd0ff9bb 100644 --- a/tensorboard/components/tf_backend/router.ts +++ b/tensorboard/components/tf_backend/router.ts @@ -75,6 +75,13 @@ namespace tf_backend { return _router; } + /** + * @return {string} the experiment ID for the currently loaded page + */ + export function getExperimentId() { + return new URLSearchParams(window.location.search).get('experiment') || ''; + } + /** * Set the global router, to be returned by future calls to `getRouter`. * You may wish to invoke this if you are running a demo server with a diff --git a/tensorboard/components/tf_backend/runsStore.ts b/tensorboard/components/tf_backend/runsStore.ts index 5c1554bf97..646bd68a75 100644 --- a/tensorboard/components/tf_backend/runsStore.ts +++ b/tensorboard/components/tf_backend/runsStore.ts @@ -17,7 +17,7 @@ namespace tf_backend { private _runs: string[] = []; load() { - const url = getRouter().runs(this._getExperimentId()); + const url = tf_backend.getRouter().runs(tf_backend.getExperimentId()); return this.requestManager.request(url).then((newRuns) => { if (!_.isEqual(this._runs, newRuns)) { this._runs = newRuns; @@ -26,12 +26,6 @@ namespace tf_backend { }); } - _getExperimentId() { - return ( - new URLSearchParams(window.location.search).get('experiment') || '' - ); - } - /** * Get the current list of runs. If no data is available, this will be * an empty array (i.e., there is no distinction between "no runs" and diff --git a/tensorboard/plugins/scalar/tf_scalar_dashboard/tf-scalar-dashboard.html b/tensorboard/plugins/scalar/tf_scalar_dashboard/tf-scalar-dashboard.html index 415449169e..efa67b49bf 100644 --- a/tensorboard/plugins/scalar/tf_scalar_dashboard/tf-scalar-dashboard.html +++ b/tensorboard/plugins/scalar/tf_scalar_dashboard/tf-scalar-dashboard.html @@ -314,7 +314,7 @@

No scalar data was found.

.pluginRoute( 'scalars', '/tags', - new URLSearchParams({experiment: this._getExperimentId()}) + new URLSearchParams({experiment: tf_backend.getExperimentId()}) ); return this._requestManager.request(url).then((runToTagInfo) => { if (_.isEqual(runToTagInfo, this._runToTagInfo)) { @@ -351,7 +351,7 @@

No scalar data was found.

selectedRuns, query ); - const experiment = this._getExperimentId(); + const experiment = tf_backend.getExperimentId(); categories.forEach((category) => { category.items = category.items.map((item) => ({ tag: item.tag, @@ -361,12 +361,6 @@

No scalar data was found.

this.updateArrayProp('_categories', categories, this._getCategoryKey); }, - _getExperimentId() { - return ( - new URLSearchParams(window.location.search).get('experiment') || '' - ); - }, - _tagMetadata(category, runToTagsInfo, item) { const tag = item.tag; const runToTagInfo = {};