hparams: encapsulate direct multiplexer access #3417
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: Until now, `context.data_provider` was provided unless `generic_data` was explicitly set to `false`. Now, it’s provided in all cases (except the legacy DB modes). This was always expected to be safe, and has been live by default since TensorBoard 1.15.0. This is needed to write plugins that unconditionally assume that a data provider is available. A consequence of this is that `is_active`’s use of the experimental `list_plugins` method is now always on as well. This has been on by default for two months, but not yet in any release (other than the upcoming TensorBoard 2.2.0), so it’s slightly riskier, but it’s also expected to be safe. Test Plan: Run TensorBoard with `--generic_data false`. If pointing at an empty logdir, no plugins are shown as active. If pointing at a full plugin, all appropriate plugins are shown as active. The debugger plugin is still marked as active if `--debugger_port` is specified and as inactive if no relevant flags are given, so the fallback `is_active` calls are still working. wchargin-branch: backend-always-data-provider
Summary: Until now, the hparams plugin has been active only when hparams data is present *and* the scalars plugin is loaded and active. This cross-plugin dependency is unique and doesn’t fit well into the data provider world, so we remove it. The hparams plugin is now active iff hparams data is available, via the standard `data_plugin_names` API. Correspondingly, we loosen the error handling in parts of the plugin that rely on the scalars plugin being active. As long as the scalars plugin is included in the TensorBoard instance, this should not be a problem: TensorBoard core loads all plugins before accepting requests to any of them. If the hparams plugin is used in an installation with no scalars plugin, the errors will now be 404s instead of 500s. This seems acceptable, as running without a scalars plugin is not a common pattern. Test Plan: The hparams plugin is still marked as active in logdirs that contain hparams data and inactive in logdirs that don’t. wchargin-branch: hparams-data-provider-is-active wchargin-source: 19dedbb932881bbdb471b4d3d9796be47d1e8956
Summary: The hparams plugin makes three kinds of queries against the multiplexer: listing hparams metadata, listing scalars metadata, and reading scalar points. This commit encapsulates each such operation in a method so that we can migrate them to use data provider APIs. For now, the underlying implementations are unchanged. Test Plan: Running `git grep '\.multiplexer' tensorboard/plugins/hparams/` shows only matches in `backend_context.py` and tests. Existing tests pass, and all three views of the hparams plugin, including embedded scalar charts and the data download links, still work with and without `--generic_data true` (though in all cases the plugin actually still reads directly from the multiplexer). wchargin-branch: hparams-encapsulate-mux wchargin-source: a583e4f940723bd01dc433eb83e24776b686dc44
stephanwlee
approved these changes
Mar 23, 2020
wchargin
changed the base branch from
wchargin-hparams-data-provider-is-active
to
master
Summary: Prior to this change, `read_scalars` (resp. `read_tensors`) delegated to `list_scalars` (resp. `list_tensors`) to find the set of time series to read. This is slower than it might sound, because `list_scalars` itself needs to scan over all relevant `multiplexer.Tensors` to identify `max_step` and `max_wall_time`, which are thrown away by `read_scalars`. (That `list_scalars` needs this full scan at all is its own issue; ideally, these would be memoized onto the event multiplexer.) When a `RunTagFilter` specifying a single run and tag is given, we optimize further by requesting individual `SummaryMetadata` rather than paring down `AllSummaryMetadata`. Resolves a comment of @nfelt on #2980: <#2980 (comment)> Test Plan: When applied on top of #3419, `:list_session_groups_test` improves from taking 11.1 seconds to taking 6.6 seconds on my machine. This doesn’t seem to fully generalize; I see only ~13% speedups in a microbenchmark that hammers `read_scalars` on a logdir with all the demo data, but the improvement on that test is important. wchargin-branch: data-read-without-list wchargin-source: bc728c60dcb0039a6f802eaf154205b7161e4796
wchargin
changed the base branch from
master
to
wchargin-data-read-without-list
wchargin-branch: hparams-encapsulate-mux wchargin-source: 118c7f3a1c7c29f5983b3ad2547669d9342e6091
wchargin-branch: data-read-without-list wchargin-source: d768ced329672f2b307bd25681f111ebe1b929a8
wchargin-branch: data-read-without-list wchargin-source: d768ced329672f2b307bd25681f111ebe1b929a8
wchargin-branch: hparams-encapsulate-mux wchargin-source: fb03f6591420b8a6f7f150dd4e30dd593d5f111e
wchargin-branch: data-blob-sequence-tests wchargin-source: 664b9b53b60a76eacbd85ecca3335e62c172acf0
wchargin-branch: data-read-without-list wchargin-source: 674ec15dd632579245b7a4aed36a8cc654928123
wchargin-branch: hparams-encapsulate-mux wchargin-source: f1350f44bdb92a4f70430f30e54023c27ac42ca4
wchargin
changed the base branch from
wchargin-data-read-without-list
to
master
wchargin-branch: hparams-encapsulate-mux wchargin-source: 41defc29c91d4cdba861687177319fadc84247af
Contributor
Author
|
@googlebot wake up -_- |
bileschi
pushed a commit
to bileschi/tensorboard
that referenced
this pull request
Apr 15, 2020
Summary: The hparams plugin makes three kinds of queries against the multiplexer: listing hparams metadata, listing scalars metadata, and reading scalar points. This commit encapsulates each such operation in a method so that we can migrate them to use data provider APIs. For now, the underlying implementations are unchanged. Test Plan: Running `git grep '\.multiplexer' tensorboard/plugins/hparams/` shows only matches in `backend_context.py` and tests. Existing tests pass, and all three views of the hparams plugin, including embedded scalar charts and the data download links, still work with and without `--generic_data true` (though in all cases the plugin actually still reads directly from the multiplexer). wchargin-branch: hparams-encapsulate-mux
bileschi
pushed a commit
that referenced
this pull request
Apr 15, 2020
Summary: The hparams plugin makes three kinds of queries against the multiplexer: listing hparams metadata, listing scalars metadata, and reading scalar points. This commit encapsulates each such operation in a method so that we can migrate them to use data provider APIs. For now, the underlying implementations are unchanged. Test Plan: Running `git grep '\.multiplexer' tensorboard/plugins/hparams/` shows only matches in `backend_context.py` and tests. Existing tests pass, and all three views of the hparams plugin, including embedded scalar charts and the data download links, still work with and without `--generic_data true` (though in all cases the plugin actually still reads directly from the multiplexer). wchargin-branch: hparams-encapsulate-mux
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
The hparams plugin makes three kinds of queries against the multiplexer:
listing hparams metadata, listing scalars metadata, and reading scalar
points. This commit encapsulates each such operation in a method so that
we can migrate them to use data provider APIs. For now, the underlying
implementations are unchanged.
Test Plan:
Running
git grep '\.multiplexer' tensorboard/plugins/hparams/showsonly matches in
backend_context.pyand tests. Existing tests pass, andall three views of the hparams plugin, including embedded scalar charts
and the data download links, still work with and without
--generic_data true(though in all cases the plugin actually still reads directly fromthe multiplexer).
wchargin-branch: hparams-encapsulate-mux