From c3457040a0a41c0dd7fea87ba7ac6309346b77cb Mon Sep 17 00:00:00 2001 From: William Chargin Date: Fri, 6 Sep 2019 13:08:20 -0700 Subject: [PATCH 1/5] data: make `ScalarTimeSeries` and `ScalarDatum` value types Test Plan: Unit tests included. wchargin-branch: data-value-types --- tensorboard/data/provider.py | 38 +++++++++++++++++++++++++++++++ tensorboard/data/provider_test.py | 36 +++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/tensorboard/data/provider.py b/tensorboard/data/provider.py index e98253d520..f56c45374c 100644 --- a/tensorboard/data/provider.py +++ b/tensorboard/data/provider.py @@ -217,6 +217,30 @@ def description(self): def display_name(self): return self._display_name + def __eq__(self, other): + if not isinstance(other, ScalarTimeSeries): + return False + if self._max_step != other._max_step: + return False + if self._max_wall_time != other._max_wall_time: + return False + if self._plugin_content != other._plugin_content: + return False + if self._description != other._description: + return False + if self._display_name != other._display_name: + return False + return True + + def __hash__(self): + return hash(( + self._max_step, + self._max_wall_time, + self._plugin_content, + self._description, + self._display_name, + )) + def __repr__(self): return "ScalarTimeSeries(%s)" % ", ".join(( "max_step=%r" % (self._max_step,), @@ -257,6 +281,20 @@ def wall_time(self): def value(self): return self._value + def __eq__(self, other): + if not isinstance(other, ScalarDatum): + return False + if self._step != other._step: + return False + if self._wall_time != other._wall_time: + return False + if self._value != other._value: + return False + return True + + def __hash__(self): + return hash((self._step, self._wall_time, self._value)) + def __repr__(self): return "ScalarDatum(%s)" % ", ".join(( "step=%r" % (self._step,), diff --git a/tensorboard/data/provider_test.py b/tensorboard/data/provider_test.py index 5db931f83a..711ad5bbc2 100644 --- a/tensorboard/data/provider_test.py +++ b/tensorboard/data/provider_test.py @@ -63,6 +63,24 @@ def test_repr(self): self.assertIn(repr(x.description), repr_) self.assertIn(repr(x.display_name), repr_) + def test_eq(self): + x1 = provider.ScalarTimeSeries(77, 1234.5, b"\x12", "one", "two") + x2 = provider.ScalarTimeSeries(77, 1234.5, b"\x12", "one", "two") + x3 = provider.ScalarTimeSeries(66, 4321.0, b"\x7F", "hmm", "hum") + self.assertEqual(x1, x2) + self.assertNotEqual(x1, x3) + self.assertNotEqual(x1, object()) + + def test_hash(self): + x1 = provider.ScalarTimeSeries(77, 1234.5, b"\x12", "one", "two") + x2 = provider.ScalarTimeSeries(77, 1234.5, b"\x12", "one", "two") + x3 = provider.ScalarTimeSeries(66, 4321.0, b"\x7F", "hmm", "hum") + self.assertEqual(hash(x1), hash(x2)) + # The next check is technically not required by the `__hash__` + # contract, but _should_ pass; failure on this assertion would at + # least warrant some scrutiny. + self.assertNotEqual(hash(x1), hash(x3)) + class ScalarDatumTest(tb_test.TestCase): def test_repr(self): @@ -72,6 +90,24 @@ def test_repr(self): self.assertIn(repr(x.wall_time), repr_) self.assertIn(repr(x.value), repr_) + def test_eq(self): + x1 = provider.ScalarDatum(step=12, wall_time=0.25, value=1.25) + x2 = provider.ScalarDatum(step=12, wall_time=0.25, value=1.25) + x3 = provider.ScalarDatum(step=23, wall_time=3.25, value=-0.5) + self.assertEqual(x1, x2) + self.assertNotEqual(x1, x3) + self.assertNotEqual(x1, object()) + + def test_hash(self): + x1 = provider.ScalarDatum(step=12, wall_time=0.25, value=1.25) + x2 = provider.ScalarDatum(step=12, wall_time=0.25, value=1.25) + x3 = provider.ScalarDatum(step=23, wall_time=3.25, value=-0.5) + self.assertEqual(hash(x1), hash(x2)) + # The next check is technically not required by the `__hash__` + # contract, but _should_ pass; failure on this assertion would at + # least warrant some scrutiny. + self.assertNotEqual(hash(x1), hash(x3)) + class RunTagFilterTest(tb_test.TestCase): def test_defensive_copy(self): From 0b69ad50768e6cdb0cff1d49922cefb803cbe5b4 Mon Sep 17 00:00:00 2001 From: William Chargin Date: Mon, 9 Sep 2019 15:42:52 -0700 Subject: [PATCH 2/5] core: remove unused `mode` on `/data/environment` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: The `data_location` field will soon be made pluggable on the data provider, which will muddy the semantics of the `mode` field. Given that it’s unused, we can just remove it. Test Plan: There are no other uses of the removed `getMode` method: $ git grep -ho 'getMode\w*' | sort -u getModelClass getModelHeaderClass getModelName getModelName_ The frontend still seems to work fine in both logdir and DB modes. wchargin-branch: remove-environ-mode --- tensorboard/components/tf_backend/environmentStore.ts | 4 ---- tensorboard/http_api.md | 7 ++++--- tensorboard/plugins/core/core_plugin.py | 1 - 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/tensorboard/components/tf_backend/environmentStore.ts b/tensorboard/components/tf_backend/environmentStore.ts index 5c44c60567..8b94a2556c 100644 --- a/tensorboard/components/tf_backend/environmentStore.ts +++ b/tensorboard/components/tf_backend/environmentStore.ts @@ -46,10 +46,6 @@ namespace tf_backend { return this.environment ? this.environment.dataLocation : ''; } - public getMode(): Mode { - return this.environment ? this.environment.mode : null; - } - public getWindowTitle(): string { return this.environment ? this.environment.windowTitle : ''; } diff --git a/tensorboard/http_api.md b/tensorboard/http_api.md index f9b93205f8..b84813d9f2 100644 --- a/tensorboard/http_api.md +++ b/tensorboard/http_api.md @@ -150,14 +150,15 @@ Example response: ## `data/environment` -Returns environment in which the TensorBoard app is running. Possible values of -the `mode` are: `db` and `logdir`. +Returns environment in which the TensorBoard app is running. + +The `data_location` is a user-readable string describing the source from which +TensorBoard is reading data, such as a directory on disk or a SQLite database. Example response: { "window_title": "Custom Name", - "mode": "db", "data_location": "sqlite:/Users/tbuser/some_session.sqlite" } diff --git a/tensorboard/plugins/core/core_plugin.py b/tensorboard/plugins/core/core_plugin.py index ebb76c285a..a46f658943 100644 --- a/tensorboard/plugins/core/core_plugin.py +++ b/tensorboard/plugins/core/core_plugin.py @@ -124,7 +124,6 @@ def _serve_environment(self, request): request, { 'data_location': self._logdir or self._db_uri, - 'mode': 'db' if self._db_uri else 'logdir', 'window_title': self._window_title, }, 'application/json') From 2412beaa93f15fd6b3cac243ff219c9939705618 Mon Sep 17 00:00:00 2001 From: William Chargin Date: Mon, 9 Sep 2019 16:06:34 -0700 Subject: [PATCH 3/5] [update patch] wchargin-branch: remove-environ-mode wchargin-source: ce7609bebd9c2a9d759829abdda95202619bc9cb --- tensorboard/components/tf_backend/environmentStore.ts | 7 ------- tensorboard/plugins/core/core_plugin_test.py | 11 ----------- 2 files changed, 18 deletions(-) diff --git a/tensorboard/components/tf_backend/environmentStore.ts b/tensorboard/components/tf_backend/environmentStore.ts index 8b94a2556c..961a5b9ff6 100644 --- a/tensorboard/components/tf_backend/environmentStore.ts +++ b/tensorboard/components/tf_backend/environmentStore.ts @@ -13,14 +13,8 @@ See the License for the specific language governing permissions and limitations under the License. ==============================================================================*/ namespace tf_backend { - export enum Mode { - DB, - LOGDIR, - } - interface Environment { dataLocation: string; - mode: Mode; windowTitle: string; } @@ -32,7 +26,6 @@ namespace tf_backend { return this.requestManager.request(url).then((result) => { const environment = { dataLocation: result.data_location, - mode: result.mode == 'db' ? Mode.DB : Mode.LOGDIR, windowTitle: result.window_title, }; if (_.isEqual(this.environment, environment)) return; diff --git a/tensorboard/plugins/core/core_plugin_test.py b/tensorboard/plugins/core/core_plugin_test.py index 54fb52c792..3fafdc83f1 100644 --- a/tensorboard/plugins/core/core_plugin_test.py +++ b/tensorboard/plugins/core/core_plugin_test.py @@ -135,17 +135,6 @@ def testEnvironmentForLogdir(self): self.logdir_based_server, '/data/environment') self.assertEqual(parsed_object['data_location'], self.logdir) - def testEnvironmentForModeForDbServer(self): - """Tests environment route that returns the mode for db based server.""" - parsed_object = self._get_json(self.db_based_server, '/data/environment') - self.assertEqual(parsed_object['mode'], 'db') - - def testEnvironmentForModeForLogServer(self): - """Tests environment route that returns the mode for logdir based server.""" - parsed_object = self._get_json( - self.logdir_based_server, '/data/environment') - self.assertEqual(parsed_object['mode'], 'logdir') - def testEnvironmentForWindowTitle(self): """Test that the environment route correctly returns the window title.""" parsed_object_db = self._get_json( From 9fa66df9f86d8daa929e5b13209cf7261ad40615 Mon Sep 17 00:00:00 2001 From: William Chargin Date: Mon, 9 Sep 2019 16:30:20 -0700 Subject: [PATCH 4/5] data: expose `data_location` integration point MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Generic data providers can now control the “data location” string in the TensorBoard UI, which appears at the bottom of the runs selector: ![Screenshot of the runs selector and its data location string][ss] As this is only a cosmetic change, it’s provided as an optional method on the `DataProvider` interface. [ss]: https://user-images.githubusercontent.com/4317806/64572837-cf7b7e00-d31d-11e9-8fd6-d81d36774172.png Test Plan: TensorBoard still seems to work properly with `--generic_data` set to `false`, `auto`, or `true`, in both logdir and DB modes. Conspicuously changing the `MultiplexerDataProvider` to always `return "tada"` induces the appropriate change in the frontend, so the wiring is hooked up. wchargin-branch: generic-data-location wchargin-source: 34124f25978fa827704b106283c03c00c653198c --- tensorboard/backend/application.py | 4 +++- .../backend/event_processing/data_provider.py | 9 ++++++++- .../event_processing/data_provider_test.py | 12 +++++++++--- .../components/tf_backend/environmentStore.ts | 4 +++- tensorboard/components/tf_backend/router.ts | 8 ++++++-- .../components/tf_backend/test/backendTests.ts | 2 +- tensorboard/data/provider.py | 16 ++++++++++++++++ tensorboard/plugins/core/core_plugin.py | 7 ++++++- .../plugins/scalar/scalars_plugin_test.py | 2 +- 9 files changed, 53 insertions(+), 11 deletions(-) diff --git a/tensorboard/backend/application.py b/tensorboard/backend/application.py index f2f7c0e232..726922f2fd 100644 --- a/tensorboard/backend/application.py +++ b/tensorboard/backend/application.py @@ -138,7 +138,9 @@ def standard_tensorboard_wsgi(flags, plugin_loaders, assets_zip_provider): max_reload_threads=flags.max_reload_threads, event_file_active_filter=_get_event_file_active_filter(flags)) if flags.generic_data != 'false': - data_provider = event_data_provider.MultiplexerDataProvider(multiplexer) + data_provider = event_data_provider.MultiplexerDataProvider( + multiplexer, flags.logdir + ) if reload_interval >= 0: # We either reload the multiplexer once when TensorBoard starts up, or we diff --git a/tensorboard/backend/event_processing/data_provider.py b/tensorboard/backend/event_processing/data_provider.py index ef60232058..0b354aa8ec 100644 --- a/tensorboard/backend/event_processing/data_provider.py +++ b/tensorboard/backend/event_processing/data_provider.py @@ -29,14 +29,17 @@ class MultiplexerDataProvider(provider.DataProvider): - def __init__(self, multiplexer): + def __init__(self, multiplexer, logdir): """Trivial initializer. Args: multiplexer: A `plugin_event_multiplexer.EventMultiplexer` (note: not a boring old `event_multiplexer.EventMultiplexer`). + logdir: The log directory from which data is being read. Only used + cosmetically. Should be a `str`. """ self._multiplexer = multiplexer + self._logdir = logdir def _test_run_tag(self, run_tag_filter, run, tag): runs = run_tag_filter.runs @@ -53,6 +56,10 @@ def _get_first_event_timestamp(self, run_name): except ValueError as e: return None + def data_location(self, experiment_id): + del experiment_id # ignored + return str(self._logdir) + def list_runs(self, experiment_id): del experiment_id # ignored for now return [ diff --git a/tensorboard/backend/event_processing/data_provider_test.py b/tensorboard/backend/event_processing/data_provider_test.py index 6c8b6c6bf3..ca0914a386 100644 --- a/tensorboard/backend/event_processing/data_provider_test.py +++ b/tensorboard/backend/event_processing/data_provider_test.py @@ -75,7 +75,13 @@ def create_multiplexer(self): return multiplexer def create_provider(self): - return data_provider.MultiplexerDataProvider(self.create_multiplexer()) + multiplexer = self.create_multiplexer() + return data_provider.MultiplexerDataProvider(multiplexer, self.logdir) + + def test_data_location(self): + provider = self.create_provider() + result = provider.data_location(experiment_id="unused") + self.assertEqual(result, self.logdir) def test_list_runs(self): # We can't control the timestamps of events written to disk (without @@ -102,7 +108,7 @@ def FirstEventTimestamp(multiplexer, run): return result multiplexer = FakeMultiplexer() - provider = data_provider.MultiplexerDataProvider(multiplexer) + provider = data_provider.MultiplexerDataProvider(multiplexer, "fake_logdir") result = provider.list_runs(experiment_id="unused") self.assertItemsEqual(result, [ base_provider.Run(run_id=run, run_name=run, start_time=start_time) @@ -164,7 +170,7 @@ def test_list_scalars_filters(self): def test_read_scalars(self): multiplexer = self.create_multiplexer() - provider = data_provider.MultiplexerDataProvider(multiplexer) + provider = data_provider.MultiplexerDataProvider(multiplexer, self.logdir) run_tag_filter = base_provider.RunTagFilter( runs=["waves", "polynomials", "unicorns"], diff --git a/tensorboard/components/tf_backend/environmentStore.ts b/tensorboard/components/tf_backend/environmentStore.ts index 961a5b9ff6..419027a96a 100644 --- a/tensorboard/components/tf_backend/environmentStore.ts +++ b/tensorboard/components/tf_backend/environmentStore.ts @@ -22,7 +22,9 @@ namespace tf_backend { private environment: Environment; load() { - const url = tf_backend.getRouter().environment(); + const url = tf_backend + .getRouter() + .environment(tf_backend.getExperimentId()); return this.requestManager.request(url).then((result) => { const environment = { dataLocation: result.data_location, diff --git a/tensorboard/components/tf_backend/router.ts b/tensorboard/components/tf_backend/router.ts index 77bd0ff9bb..d6cb443f4b 100644 --- a/tensorboard/components/tf_backend/router.ts +++ b/tensorboard/components/tf_backend/router.ts @@ -14,7 +14,7 @@ limitations under the License. ==============================================================================*/ namespace tf_backend { export interface Router { - environment: () => string; + environment: (experiment?: string) => string; experiments: () => string; pluginRoute: ( pluginName: string, @@ -39,7 +39,11 @@ namespace tf_backend { dataDir = dataDir.slice(0, dataDir.length - 1); } return { - environment: () => createDataPath(dataDir, '/environment'), + environment: (experiment?: string) => { + const searchParams = new URLSearchParams(); + searchParams.set('experiment', experiment || ''); + return createDataPath(dataDir, '/environment', searchParams); + }, experiments: () => createDataPath(dataDir, '/experiments'), pluginRoute: ( pluginName: string, diff --git a/tensorboard/components/tf_backend/test/backendTests.ts b/tensorboard/components/tf_backend/test/backendTests.ts index 2e1f2fc272..9be6ae0ba2 100644 --- a/tensorboard/components/tf_backend/test/backendTests.ts +++ b/tensorboard/components/tf_backend/test/backendTests.ts @@ -97,7 +97,7 @@ namespace tf_backend { }); it('returns correct value for #environment', () => { - assert.equal(router.environment(), 'data/environment'); + assert.equal(router.environment(), 'data/environment?experiment='); }); it('returns correct value for #experiments', () => { diff --git a/tensorboard/data/provider.py b/tensorboard/data/provider.py index f56c45374c..29bc1efa07 100644 --- a/tensorboard/data/provider.py +++ b/tensorboard/data/provider.py @@ -33,6 +33,22 @@ class DataProvider(object): downsampling strategies or domain restriction by step or wall time. """ + def data_location(self, experiment_id): + """Render a human-readable description of the data source. + + For instance, this might return a path to a directory on disk. + + The default implementation always returns the empty string. + + Args: + experiment_id: ID of enclosing experiment. + + Returns: + A string, which may be empty. + """ + return "" + + @abc.abstractmethod def list_runs(self, experiment_id): """List all runs within an experiment. diff --git a/tensorboard/plugins/core/core_plugin.py b/tensorboard/plugins/core/core_plugin.py index a46f658943..dfc06506bc 100644 --- a/tensorboard/plugins/core/core_plugin.py +++ b/tensorboard/plugins/core/core_plugin.py @@ -120,10 +120,15 @@ def _serve_environment(self, request): database (depending on which mode TensorBoard is running in). * window_title is the title of the TensorBoard web page. """ + if self._data_provider: + experiment = request.args.get('experiment', '') + data_location = self._data_provider.data_location(experiment) + else: + data_location = self._logdir or self._db_uri return http_util.Respond( request, { - 'data_location': self._logdir or self._db_uri, + 'data_location': data_location, 'window_title': self._window_title, }, 'application/json') diff --git a/tensorboard/plugins/scalar/scalars_plugin_test.py b/tensorboard/plugins/scalar/scalars_plugin_test.py index 0f5c8801ec..0a5351caeb 100644 --- a/tensorboard/plugins/scalar/scalars_plugin_test.py +++ b/tensorboard/plugins/scalar/scalars_plugin_test.py @@ -124,7 +124,7 @@ def wrapper(self, *args, **kwargs): fn(self, scalars_plugin.ScalarsPlugin(ctx), *args, **kwargs) with self.subTest('generic data provider'): flags = argparse.Namespace(generic_data='true') - provider = data_provider.MultiplexerDataProvider(multiplexer) + provider = data_provider.MultiplexerDataProvider(multiplexer, logdir) ctx = base_plugin.TBContext( flags=flags, logdir=logdir, From 028d8f305c144a0fe970f8e4499d11ae3b825ad3 Mon Sep 17 00:00:00 2001 From: William Chargin Date: Fri, 13 Sep 2019 19:42:25 -0700 Subject: [PATCH 5/5] [update patch] wchargin-branch: generic-data-location wchargin-source: 96c7f9577e32de041c8dc53e7a9ab5728c23e64e --- tensorboard/plugins/core/core_plugin.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tensorboard/plugins/core/core_plugin.py b/tensorboard/plugins/core/core_plugin.py index c76addb3e6..dfc06506bc 100644 --- a/tensorboard/plugins/core/core_plugin.py +++ b/tensorboard/plugins/core/core_plugin.py @@ -128,11 +128,7 @@ def _serve_environment(self, request): return http_util.Respond( request, { -<<<<<<< HEAD 'data_location': data_location, -======= - 'data_location': self._logdir or self._db_uri, ->>>>>>> 2ff9cf0ee69852b9465e23453699a8cf4dd04ee2 'window_title': self._window_title, }, 'application/json')