-
Notifications
You must be signed in to change notification settings - Fork 1.7k
scalars: multiplex data fetches within a tag #4050
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
065d7ca
beff777
f4ddbf7
50c7112
a80c564
aaa17f9
534702f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -225,14 +225,18 @@ export class TfScalarCard extends PolymerElement { | |
|
||
// This function is called when data is received from the backend. | ||
@property({type: Object}) | ||
_loadDataCallback: object = (scalarChart, datum, data) => { | ||
const formattedData = data.map((datum) => ({ | ||
_loadDataCallback: object = (scalarChart, item, maybeData) => { | ||
if (maybeData == null) { | ||
console.error('Failed to load data for:', item); | ||
return; | ||
} | ||
const formattedData = maybeData.map((datum) => ({ | ||
wall_time: new Date(datum[0] * 1000), | ||
step: datum[1], | ||
scalar: datum[2], | ||
})); | ||
const name = this._getSeriesNameFromDatum(datum); | ||
scalarChart.setSeriesMetadata(name, datum); | ||
const name = this._getSeriesNameFromDatum(item); | ||
scalarChart.setSeriesMetadata(name, item); | ||
scalarChart.setSeriesData(name, formattedData); | ||
scalarChart.commitChanges(); | ||
}; | ||
|
@@ -257,19 +261,56 @@ export class TfScalarCard extends PolymerElement { | |
// this.requestManager.request( | ||
// this.getDataLoadUrl({tag, run, experiment}) | ||
@property({type: Object}) | ||
requestData: RequestDataCallback<RunTagItem, ScalarDatum[]> = ( | ||
requestData: RequestDataCallback<RunTagItem, ScalarDatum[] | null> = ( | ||
items, | ||
onLoad, | ||
onFinish | ||
) => { | ||
const router = getRouter(); | ||
const baseUrl = router.pluginRoute('scalars', '/scalars'); | ||
const url = router.pluginRoute('scalars', '/scalars_multirun'); | ||
const runsByTag = new Map<string, string[]>(); | ||
for (const {tag, run} of items) { | ||
let runs = runsByTag.get(tag); | ||
if (runs == null) { | ||
runsByTag.set(tag, (runs = [])); | ||
} | ||
runs.push(run); | ||
} | ||
|
||
// Request at most this many runs at once. | ||
// | ||
// Back-of-the-envelope math: each scalar datum JSON value contains | ||
// two floats and a small-ish integer. Floats are about 18 bytes, | ||
// since f64s have -log_10(2^-53) ~= 16 digits of precision plus | ||
// decimal point and leading zero. Small-ish integers (steps) are | ||
// about 5 bytes. Add JSON overhead `[,,],` and you're looking at | ||
// about 48 bytes per datum. With standard downsampling of | ||
// 1000 points per time series, expect ~50 KB of response payload | ||
// per requested time series. | ||
// | ||
// Requesting 64 time series warrants a ~3 MB response, which seems | ||
// reasonable. | ||
const BATCH_SIZE = 64; | ||
|
||
const requestGroups = []; | ||
for (const [tag, runs] of runsByTag) { | ||
for (let i = 0; i < runs.length; i += BATCH_SIZE) { | ||
requestGroups.push({tag, runs: runs.slice(i, i + BATCH_SIZE)}); | ||
} | ||
} | ||
|
||
Promise.all( | ||
items.map((item) => { | ||
const url = addParams(baseUrl, {tag: item.tag, run: item.run}); | ||
return this.requestManager | ||
.request(url) | ||
.then((data) => void onLoad({item, data})); | ||
requestGroups.map(({tag, runs}) => { | ||
return this.requestManager.request(url, {tag, runs}).then((allData) => { | ||
for (const run of runs) { | ||
const item = {tag, run}; | ||
if (Object.prototype.hasOwnProperty.call(allData, run)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason why we have to use Object.prototype.hasOwnProperty? (i.e., why not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you have a run called > ({train: 1}).hasOwnProperty("train")
true
> ({train: 1, hasOwnProperty: 2}).hasOwnProperty("train")
Thrown:
TypeError: {(intermediate value)(intermediate value)}.hasOwnProperty is not a function
> Object.prototype.hasOwnProperty.call({train: 1, hasOwnProperty: 2}, "train")
true Yeah. I prefer to write this kind of code defensively—even if it seems There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One day https://github.com/hasOwnProperty will show up in a JSON There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. Thanks. |
||
onLoad({item, data: allData[run]}); | ||
} else { | ||
onLoad({item, data: null}); | ||
} | ||
} | ||
}); | ||
}) | ||
).finally(() => void onFinish()); | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the contract if I violate this constraint? (0 or 2+
tag
)? Unlike the zero lengthruns
case, it looks like we are throwing 400.Also, do you think it is a good idea to perhaps write an example of the request?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Zero-length
runs
is fine; it just returns an empty object. Missingtag
is a 400. Duplicatetag
s will silently pick one of them(probably last?). I can make that an error, though; probably a good
idea. Thanks.
Yep. Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I expected to see FormData but this works too :)