Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions tensorboard/components/tf_backend/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace tf_backend {
params?: URLSearchParams
) => string;
pluginsListing: () => string;
runs: () => string;
runs: (experiment?: string) => string;
runsForExperiment: (id: tf_backend.ExperimentId) => string;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This machinery is unrelated; I’ll remove it in a future change.

I didn’t want to reuse it because it assumes some things about both
inputs (e.g., type ExperimentId = number) and outputs (the output of
the /experiment_runs route is not just a list of string names) that
may not be what we want.

}

Expand Down Expand Up @@ -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,
Expand All @@ -71,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
Expand Down
2 changes: 1 addition & 1 deletion tensorboard/components/tf_backend/runsStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace tf_backend {
private _runs: string[] = [];

load() {
const url = getRouter().runs();
const url = tf_backend.getRouter().runs(tf_backend.getExperimentId());
return this.requestManager.request(url).then((newRuns) => {
if (!_.isEqual(this._runs, newRuns)) {
this._runs = newRuns;
Expand Down
10 changes: 5 additions & 5 deletions tensorboard/components/tf_backend/test/backendTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down
4 changes: 2 additions & 2 deletions tensorboard/plugins/core/core_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
11 changes: 6 additions & 5 deletions tensorboard/plugins/scalar/scalars_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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]),
Expand Down Expand Up @@ -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
Expand All @@ -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', '')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason to have this default to empty instead of None?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The experiment ID is required for any calls to the data provider API
(even though our current implementation doesn’t strictly validate
this—after this PR, maybe it should), so it would have to be converted
from empty string to None at some point. It seemed cleanest to me to
reduce the space of input types as early as possible. No strong opinion.

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)
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@
new URLSearchParams({
tag,
run,
experiment: experiment ? experiment.id : '',
experiment: experiment || '',
})
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,13 @@ <h3>No scalar data was found.</h3>
});
},
_fetchTags() {
const url = tf_backend.getRouter().pluginRoute('scalars', '/tags');
const url = tf_backend
.getRouter()
.pluginRoute(
'scalars',
'/tags',
new URLSearchParams({experiment: tf_backend.getExperimentId()})
);
return this._requestManager.request(url).then((runToTagInfo) => {
if (_.isEqual(runToTagInfo, this._runToTagInfo)) {
// No need to update anything if there are no changes.
Expand Down Expand Up @@ -345,10 +351,11 @@ <h3>No scalar data was found.</h3>
selectedRuns,
query
);
const experiment = tf_backend.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);
Expand Down