Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support dynamic stream metadata #493

Merged
merged 5 commits into from
Jul 9, 2019
Merged

Support dynamic stream metadata #493

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
64b289e
generate stream metadata dynamically
Jul 8, 2019
507092a
track stream count in XvizStreamBuffer
Jul 8, 2019
c34c304
lint
Jul 8, 2019
e5bae35
fix future type inference
Jul 8, 2019
cc344fa
address comments
Jul 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/api-reference/xviz-configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ Constants:
- `ALLOW_MISSING_PRIMARY_POSE` (Boolean) - Whether to render logs where there is no `/vehicle_pose`
stream. If `true`, `VEHICLE_RELATIVE` coordinates are treated as `IDENTITY`. Using this may break
certain functionalities in the `LogViewer` and `PlaybackControl`. Default `false`.
- `DYNAMIC_STREAM_METADATA` (Boolean) - Whether to automatically create metadata from streams that
are not present in `metadata.streams`

Parser hooks:

Expand Down
2 changes: 2 additions & 0 deletions modules/parser/src/config/xviz-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ const DEFAULT_XVIZ_CONFIG = {
preProcessPrimitive: primitive => primitive,
// Allow scenarios where there is no /vehicle_pose stream
ALLOW_MISSING_PRIMARY_POSE: false,
// Auto backfill missing stream metadata
DYNAMIC_STREAM_METADATA: false,

/* Deprecated configs, do not use */

Expand Down
7 changes: 7 additions & 0 deletions modules/parser/src/parsers/parse-xviz-pose.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
import {getXVIZConfig} from '../config/xviz-config';

/* eslint-disable camelcase */
export function parseXVIZPose(pose) {
Expand Down Expand Up @@ -50,5 +51,11 @@ export function parseXVIZPose(pose) {
});
}

if (getXVIZConfig().DYNAMIC_STREAM_METADATA) {
result.__metadata = {
category: 'POSE'
};
}

return {...pose, ...result};
}
90 changes: 66 additions & 24 deletions modules/parser/src/parsers/parse-xviz-stream.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ function createPrimitiveMap() {
return result;
}

const SCALAR_TYPE = {
doubles: 'FLOAT',
int32s: 'INT32',
bools: 'BOOL',
strings: 'STRING'
};

/* eslint-disable max-depth, max-statements, complexity, camelcase */
// Handle stream-sliced data, via the ETL flow.
export function parseXVIZStream(data, convertPrimitive) {
Expand Down Expand Up @@ -82,7 +89,7 @@ export function parseXVIZStream(data, convertPrimitive) {
* data to UI elements.
*/
export function parseStreamPrimitive(primitives, streamName, time, convertPrimitive) {
const {OBJECT_STREAM, preProcessPrimitive} = getXVIZConfig();
const {OBJECT_STREAM, preProcessPrimitive, DYNAMIC_STREAM_METADATA} = getXVIZConfig();
const PRIMITIVE_SETTINGS =
getXVIZConfig().currentMajorVersion === 1 ? XVIZPrimitiveSettingsV1 : XVIZPrimitiveSettingsV2;

Expand Down Expand Up @@ -172,18 +179,35 @@ export function parseStreamPrimitive(primitives, streamName, time, convertPrimit
primitiveMap.pointCloud = pointCloud;
primitiveMap.time = time;

if (DYNAMIC_STREAM_METADATA) {
primitiveMap.__metadata = {
category: 'PRIMITIVE',
primitive_type: primType
};
}

return primitiveMap;
}

/* Processes the futures and converts the
* data to UI elements.
*/
export function parseStreamFutures(objects, streamName, time, convertPrimitive) {
const {currentMajorVersion} = getXVIZConfig();
const {currentMajorVersion, DYNAMIC_STREAM_METADATA} = getXVIZConfig();

const result =
currentMajorVersion === 1
? parseStreamFuturesV1(objects, streamName, time, convertPrimitive)
: parseStreamFuturesV2(objects, streamName, time, convertPrimitive);

if (DYNAMIC_STREAM_METADATA) {
result.__metadata = {
category: 'FUTURE_INSTANCE',
primitive_type: result.lookAheads[0][0] && result.lookAheads[0][0].type
};
}

return currentMajorVersion === 1
? parseStreamFuturesV1(objects, streamName, time, convertPrimitive)
: parseStreamFuturesV2(objects, streamName, time, convertPrimitive);
return result;
}

export function parseStreamFuturesV1(objects, streamName, time, convertPrimitive) {
Expand Down Expand Up @@ -267,11 +291,21 @@ export function parseStreamFuturesV2(objects, streamName, time, convertPrimitive
* data to UI elements.
*/
export function parseStreamVariable(objects, streamName, time) {
const {currentMajorVersion} = getXVIZConfig();
const {currentMajorVersion, DYNAMIC_STREAM_METADATA} = getXVIZConfig();

const result =
currentMajorVersion === 1
? parseStreamVariableV1(objects, streamName, time)
: parseStreamVariableV2(objects, streamName, time);

if (DYNAMIC_STREAM_METADATA) {
result.__metadata = {
category: 'VARIABLE',
scalar_type: Array.isArray(result.variable) && result.variable[0] && result.variable[0].type
};
}

return currentMajorVersion === 1
? parseStreamVariableV1(objects, streamName, time)
: parseStreamVariableV2(objects, streamName, time);
return result;
}

export function parseStreamVariableV1(objects, streamName, time) {
Expand Down Expand Up @@ -318,15 +352,11 @@ export function parseStreamVariableV2(objects, streamName, time) {
return null;
}

const datum = {
values: valueData.values
};

if (base && base.object_id) {
datum.id = base.object_id;
valueData.id = base.object_id;
}

return datum;
return valueData;
})
.filter(Boolean);

Expand All @@ -347,26 +377,23 @@ export function parseStreamTimeSeries(seriesArray, streamBlackList) {
return null;
}

const ValueTypes = ['doubles', 'int32s', 'bools', 'strings'];

function getVariableData(valuesObject) {
// Primitives have the type as the first key
const keys = Object.keys(valuesObject);

for (const type of keys) {
if (ValueTypes.includes(type)) {
return {type, values: valuesObject[type]};
for (const key in valuesObject) {
if (key in SCALAR_TYPE) {
return {type: SCALAR_TYPE[key], values: valuesObject[key]};
}
}

// TODO(twojtasz): a more informative error path that doesn't abort processing
return {};
return null;
}

function parseStreamTimeSeriesV2(seriesArray, streamBlackList) {
if (!Array.isArray(seriesArray)) {
return {};
}
const {DYNAMIC_STREAM_METADATA} = getXVIZConfig();

const timeSeriesStreams = {};
seriesArray.forEach(timeSeriesEntry => {
Expand Down Expand Up @@ -394,6 +421,13 @@ function parseStreamTimeSeriesV2(seriesArray, streamBlackList) {
} else {
timeSeriesStreams[streamName] = entry;
}

if (DYNAMIC_STREAM_METADATA) {
entry.__metadata = {
category: 'TIME_SERIES',
scalar_type: valueData.type
};
}
}
});

Expand Down Expand Up @@ -561,5 +595,13 @@ function joinObjectPointCloudsToTypedArrays(objects) {
}

export function parseStreamUIPrimitives(components, streamName, time) {
return Object.assign({time}, components);
const result = Object.assign({time}, components);

if (getXVIZConfig().DYNAMIC_STREAM_METADATA) {
result.__metadata = {
category: 'UI_PRIMITIVE'
};
}

return result;
}
3 changes: 3 additions & 0 deletions modules/parser/src/synchronizers/xviz-stream-buffer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ export default class XVIZStreamBuffer {
this.videos = {};
/* Update counter */
this.lastUpdate = 0;
/* Track the number of unique streams */
this.streamCount = 0;

this.hasBuffer = this.hasBuffer.bind(this);
}
Expand Down Expand Up @@ -217,6 +219,7 @@ export default class XVIZStreamBuffer {
for (const streamName in timeslice.streams) {
if (!streams[streamName]) {
streams[streamName] = new Array(timeslices.length);
this.streamCount++;
}
}
for (const streamName in timeslice.videos) {
Expand Down
8 changes: 8 additions & 0 deletions test/modules/parser/parsers/parse-xviz-pose.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
// limitations under the License.

/* eslint-disable camelcase */
import {setXVIZConfig} from '@xviz/parser';
import {parseXVIZPose} from '@xviz/parser/parsers/parse-xviz-pose';
import {resetXVIZConfigAndSettings} from '../config/config-utils';

import tape from 'tape-catch';

Expand Down Expand Up @@ -43,8 +45,14 @@ function parsedPoseCheck(t, parsedPose, xvizPose) {
}

tape('parseXVIZPose#simple', t => {
resetXVIZConfigAndSettings();
setXVIZConfig({DYNAMIC_STREAM_METADATA: true});

const result = parseXVIZPose(testXVIZPose);
parsedPoseCheck(t, result, testXVIZPose);

t.is(result.__metadata.category, 'POSE', 'metadata generated');

t.end();
});

Expand Down
31 changes: 27 additions & 4 deletions test/modules/parser/parsers/parse-xviz-stream.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,14 @@ tape('parseStreamTimeSeries#simple', t => {

testData.forEach(d => schemaValidator.validate('core/timeseries_state', d));

const result = parseStreamTimeSeries(testData, new Map());
let result = parseStreamTimeSeries(testData, new Map());
t.deepEquals(result, expected, 'time_series parsed properly');

setXVIZConfig({DYNAMIC_STREAM_METADATA: true});
result = parseStreamTimeSeries(testData, new Map());
t.is(result['/test/doubles'].__metadata.category, 'TIME_SERIES', 'metadata generated');
t.is(result['/test/doubles'].__metadata.scalar_type, 'FLOAT', 'metadata generated');

t.end();
});

Expand Down Expand Up @@ -133,15 +138,19 @@ tape('parseStreamVariable#simple v2', t => {
time: 1001,
variable: [
{
type: 'FLOAT',
values: [10, 11, 12]
},
{
type: 'INT32',
values: [10, 11, 12]
},
{
type: 'BOOL',
values: [true, false, true]
},
{
type: 'STRING',
values: ['one', 'two', 'three'],
id: '123'
}
Expand All @@ -153,6 +162,10 @@ tape('parseStreamVariable#simple v2', t => {
const result = parseStreamVariable(testData, '/test', time);
t.deepEquals(result, expected, 'variables parsed properly');

setXVIZConfig({DYNAMIC_STREAM_METADATA: true});
const {__metadata} = parseStreamVariable(testData, '/test', time);
t.is(__metadata.category, 'VARIABLE', 'metadata generated');

Copy link
Contributor

Choose a reason for hiding this comment

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

Should scalar_type be verified here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How does scalar_type in metadata work when there are multiple types in this stream?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally there shouldn't be.

The intention was that the multiple elements only really make sense if you assign an object_id so you can associate the values with something meaningful (if they share the stream id like /object/speed_prediction)
Otherwise I don't know how you interpret or use the elements, like if I have /object/velocity_prediction with multiple entries but w/o an object_id for each one, I don't know how to make sense of the data then.

The same issue fundamentally happens with primitives in that we assume each primitive is of a single primitive_type. Like you can't have /object/ufo as both circles and polygons. It doesn't look like the Schema enforces this restriction, but i'm pretty sure our code won't really handle them right.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am saying, either the relationship is implicit, and the variable would have only 1 entry, or it must be explicit with an object_id and multiple entries. Otherwise it is hard to see how the data is useful.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be fine with taking the type of the first entry. It is likely what we do elsewhere if necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like getVariableData() returns the first entry if finds too

t.end();
});

Expand Down Expand Up @@ -181,6 +194,10 @@ tape('parseStreamUIPrimitives#simple v2', t => {
const result = parseStreamUIPrimitives(testData, '/test', time);
t.deepEquals(result, expected, 'variables parsed properly');

setXVIZConfig({DYNAMIC_STREAM_METADATA: true});
const {__metadata} = parseStreamUIPrimitives(testData, '/test', time);
t.is(__metadata.category, 'UI_PRIMITIVE', 'metadata generated');

t.end();
});

Expand Down Expand Up @@ -233,6 +250,10 @@ tape('parseStreamVariable#simple v1', t => {
t.deepEquals(result, testCase.expected, `variables type ${testCase.name} parsed properly`);
});

setXVIZConfig({DYNAMIC_STREAM_METADATA: true});
const {__metadata} = parseStreamVariable(testSet[0].xviz, '/test', time);
t.is(__metadata.category, 'VARIABLE', 'metadata generated');

t.end();
});

Expand Down Expand Up @@ -278,7 +299,7 @@ tape('parseXVIZStream#variable no-data entries', t => {

tape('parseXVIZStream#primitive no-data entries', t => {
resetXVIZConfigAndSettings();
setXVIZConfig({currentMajorVersion: 1});
setXVIZConfig({currentMajorVersion: 1, DYNAMIC_STREAM_METADATA: true});

const data = [
{
Expand Down Expand Up @@ -314,7 +335,8 @@ tape('parseXVIZStream#primitive no-data entries', t => {
pointCloud: null,
images: [],
components: [],
time: 100.5425
time: 100.5425,
__metadata: {category: 'PRIMITIVE', primitive_type: 'polygon2d'}
Copy link
Contributor

Choose a reason for hiding this comment

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

polygon2d is only xviz v1, not sure if this makes sense outside of v2. (v1 didn't have metadata)

},
{
lookAheads: [],
Expand All @@ -324,7 +346,8 @@ tape('parseXVIZStream#primitive no-data entries', t => {
pointCloud: null,
images: [],
components: [],
time: 101.84
time: 101.84,
__metadata: {category: 'PRIMITIVE', primitive_type: null}
}
];

Expand Down