Batch Table Hierarchy #171

lilleyse · 2017-01-18T19:59:17Z

For #66
Cesium implementation : CesiumGS/cesium#4625

This is a rough draft for the batch table binary.

To-do:

Batch Table Hierarchy schema

lilleyse · 2017-01-19T16:32:41Z

This is ready for review now. The Batch Table Hierarchy section in Batch Table is pretty long - maybe it belongs in its own file.

pjcozzi · 2017-01-21T18:03:28Z

@arneschilling @pmconne @jbo023 your input is very welcomed here!

pjcozzi · 2017-01-21T18:05:56Z

Direct links

pjcozzi · 2017-01-21T18:07:45Z

Can you add a TOC to TileFormats/BatchTable/README.md?

pjcozzi · 2017-01-21T18:09:06Z

Styling/README.md

@@ -46,6 +46,7 @@ Contents:
   * [Variables](#variables)
   * [Built-in Variables](#built-in-variables)
   * [Built-in Functions](#built-in-functions)
+   * [Batch Table Hierarchy](#batch-table-hierarchy)


I don't think the TOC indentation is right here; a lot of this should not be under Expressions. Can you reorganize?

It fits ok, as those sections describe expressions related to the hierarchy and point clouds. I'm fine with placing them in a different section too.

Yes, I think this should be in a top-level section, not under Expressions.

pjcozzi · 2017-01-21T18:19:43Z

Class and Feature should be precisely defined; keep in mind this is a spec, not a tutorial.

pjcozzi · 2017-01-21T18:22:39Z

Class and Feature should be precisely defined; keep in mind this is a spec, not a tutorial.

Also define Instance.

Use a diagram if it is helpful.

pjcozzi · 2017-01-21T18:24:08Z

classes is an array of objects, where each object contains the following properties:

name - the name of the class

length - the number of instances of the class

instances - metadata for the instances. This section is similar to a standard batch table; properties may be stored as an array of values or a reference to data in the binary body.

Although the schema is the authority, here and throughout, precisely define the types like the spec does elsewhere, e.g., "A string representing the name of the class"

pjcozzi · 2017-01-21T18:26:18Z

TileFormats/BatchTable/README.md

+* `length` - the number of instances of the class
+* `instances` - metadata for the instances. This section is similar to a standard batch table; properties may be stored as an array of values or a reference to data in the binary body.
+
+`instancesLength` is the number of instances. In this example `instancesLength` equals `batchLength`, however this is not always the case (as will be seen below).


Precisely say why or link to why. The spec needs to be rigorous.

pjcozzi · 2017-01-21T18:33:57Z

TileFormats/BatchTable/README.md

+
+Another limitation of the standard batch table is the difficulty in expressing metadata hierarchies.
+
+For example consider a tile that represents a city block. The block itself contains metadata, the individual buildings contain metadata, and the building components (doors, walls, roofs) contain metadata.


Perhaps a screenshot or a series of screenshots with different features highlighted from the Sandcastle example would be useful here.

pjcozzi · 2017-01-21T18:43:29Z

The tutorial style content is good, but we need to better separate what is precisely written spec and what is an example. Using the example to define the spec is atypical.

pjcozzi · 2017-01-21T18:45:05Z

Made some tweaks in 9c2679d.

pjcozzi · 2017-01-21T18:52:15Z

Made a few more tweaks in 22a44e3.

pjcozzi · 2017-01-21T18:52:33Z

That's all my comments; this looks good. Let me know when the updates and schema are ready.

lilleyse · 2017-01-23T16:42:49Z

Thanks for the first round of reviewing.

The tutorial style content is good, but we need to better separate what is precisely written spec and what is an example. Using the example to define the spec is atypical.

Definitely agree here, I'll work on this as well as creating some diagrams.

arneschilling · 2017-02-01T10:03:24Z

"@arneschilling @pmconne @jbo023 your input is very welcomed here!"

We can definitely work with this approach, thank you very much. Something is not quite clear to me. The attribute names in the classes section seem to have prefixes, e.g. "door_name" and "building_name" for classes Door and Building. Is this a naming convention or can we use the original attribute names, in our case just "name" for each class? Adding a prefix to each would alter the way we present batch table information to the user, because we cannnot always implement a dictionary mapping these strings values to more user friendly text.

From a Logical Point of view, I have one comment.
The hierarchy ("parentIds" field) and the classes seem to be independant from each other, even though classes is embedded in "HIERARCHY". One could in fact implement classes without using the parentIds as in the first example with "Lamp", "Car" and "Tree". If somebody has a collection of independant object classes without interrelations between them, the hierarchy is not needed. On the other hand, the "parentIds" field could be used in combination with a standard batch table without classes (currently this is our implementation). Why not separate these 2 things?

lilleyse · 2017-02-01T14:20:33Z

Thanks for the feedback.

The attribute names don't have to have prefixes, this was just for example purposes. In my next round of edits I will clarify that.

If someone is using the concept of parents with the standard batch table (no classes) I think they would be better off storing parent id's as a regular batch table property and have hierarchy logic be part of the app.

pjcozzi · 2017-02-05T01:13:11Z

@lilleyse what's the latest here? Is this ready?

pmconne · 2017-02-09T18:18:47Z

Having finally gotten a chance to play around with this in earnest, I have a couple of questions:

Does this work if the features have no properties of their own, only those which they inherit from their parents? e.g.:
"classes": [
{
"name": "Wall",
"length": 3,
"instances": { /* I have no properties... */ }
}
Would it be worth optimizing for the case in which every feature has the same class ID (and - at least in our case - same number of parents), rather than having the first N entries in the classIds and parentCounts arrays be identical?

I think we may have an unusual use case - we want to use this purely for classification of features, with all the properties residing on the abstract "classification" classes. We can do that with the existing spec - redundant data notwithstanding. If the answer to question 1 is "yes" then the redundancy mentioned in question 2 will not be much of an issue.

pmconne · 2017-02-10T14:03:41Z

To answer my own question...yes, instances with no properties are fine.
This is working wonderfully - thanks.

lilleyse · 2017-03-02T20:20:06Z

Second draft updated. I still think it may be worth splitting it off into a separate file - what do you think @pjcozzi.

The batch table schema is also updated.

pjcozzi · 2017-03-08T01:01:16Z

TileFormats/BatchTable/README.md

+
+### Notes
+
+* In some cases a feature may have multiple ancestors of the same class, or ancestors of different classes with the same property names. It is up to the implemantation to decide how to handle overloaded properties.


Can you provide a concrete example here.

It is up to the implemantation to decide how to handle overloaded properties.

In general, we need to avoid undefined behavior as much as possible. Are we sure we can't define reasonably behavior here?

Some examples:

An instance has two parents of the same class. Anytime the instance gets a property it will have to choose between which parent to get it from. Cesium's behavior is to get it from the first parent.

An instance may also have two different ancestors of different classes that happen to use the same property name, like id. In Cesium it will just return the property from whichever ancestor it encounters first, so similar to the situation above.

I left this open ended at first, but I think this behavior could be defined pretty easily here.

Once I started to rewrite that section I realized it may be hard to define the exact behavior, which would be equivalent to defining how the hierarchy should be traversed. I did make the wording less open-ended - so it is not allowed to return an array of values, or just do nothing.

pjcozzi · 2017-03-08T01:02:25Z

TileFormats/BatchTable/README.md

+```
+
+* The batch table hierarchy is self-contained within the tile. It is not possible to form metadata hierarchy across different tiles in the tileset.
+


Below in

TileFormats/BatchTable/figures/batch-table-hierarchy-block.png

"Labelled" should be "Labeled"

And in batch-table-hierarchy-parking-lot.png.

pjcozzi · 2017-03-08T01:03:56Z

Looks good, this is very close.

pjcozzi · 2017-03-10T19:54:17Z

Thanks!

lilleyse changed the title ~~Batch Table Binary~~ Batch Table Hierarchy Jan 18, 2017

lilleyse force-pushed the batch-table-hierarchy branch 7 times, most recently from bb0c994 to 5ce4947 Compare January 19, 2017 16:21

Add batch table binary section to BatchTable README

415bdaa

lilleyse force-pushed the batch-table-hierarchy branch from 5ce4947 to 415bdaa Compare January 19, 2017 16:28

pjcozzi added the draft 1.0 label Jan 21, 2017

pjcozzi reviewed Jan 21, 2017

View reviewed changes

Tweaks

9c2679d

Tweaks

22a44e3

Add HIERARCHY to batch table schema

e6e5482

Draft 2

f587f14

lilleyse force-pushed the batch-table-hierarchy branch from de0dacf to f38824a Compare March 2, 2017 19:41

Merge branch 'master' into batch-table-hierarchy

9c3bb6a

lilleyse force-pushed the batch-table-hierarchy branch 3 times, most recently from 5f685f1 to 5f4049a Compare March 2, 2017 19:56

Edits

48b465f

lilleyse force-pushed the batch-table-hierarchy branch from 5f4049a to 48b465f Compare March 2, 2017 20:10

Table of contents

df25e8c

lilleyse force-pushed the batch-table-hierarchy branch from 06d00e2 to df25e8c Compare March 2, 2017 20:30

Update svgs

50d1eac

pjcozzi reviewed Mar 8, 2017

View reviewed changes

pjcozzi mentioned this pull request Mar 8, 2017

Suggestion for hierarchies in batch table #66

Closed

lilleyse added 2 commits March 9, 2017 10:21

Update figures

2e4c27d

Reorganizing styling headers

5a25a5a

lilleyse force-pushed the batch-table-hierarchy branch 2 times, most recently from b8f5478 to d8eb182 Compare March 9, 2017 16:16

Clarified overloaded properties

49f99ea

lilleyse force-pushed the batch-table-hierarchy branch from d8eb182 to 49f99ea Compare March 9, 2017 16:18

pjcozzi merged commit 568c72f into master Mar 10, 2017

pjcozzi deleted the batch-table-hierarchy branch March 10, 2017 19:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Batch Table Hierarchy #171

Batch Table Hierarchy #171

lilleyse commented Jan 18, 2017 •

edited

Loading

lilleyse commented Jan 19, 2017

pjcozzi commented Jan 21, 2017

pjcozzi commented Jan 21, 2017

pjcozzi commented Jan 21, 2017

pjcozzi Jan 21, 2017

lilleyse Mar 2, 2017

pjcozzi Mar 8, 2017

pjcozzi commented Jan 21, 2017

pjcozzi commented Jan 21, 2017

pjcozzi commented Jan 21, 2017

pjcozzi Jan 21, 2017

pjcozzi Jan 21, 2017

pjcozzi commented Jan 21, 2017

pjcozzi commented Jan 21, 2017

pjcozzi commented Jan 21, 2017

pjcozzi commented Jan 21, 2017

lilleyse commented Jan 23, 2017

arneschilling commented Feb 1, 2017

lilleyse commented Feb 1, 2017

pjcozzi commented Feb 5, 2017

pmconne commented Feb 9, 2017

pmconne commented Feb 10, 2017

lilleyse commented Mar 2, 2017

pjcozzi Mar 8, 2017

lilleyse Mar 9, 2017 •

edited

Loading

lilleyse Mar 9, 2017

pjcozzi Mar 8, 2017

pjcozzi Mar 8, 2017

pjcozzi commented Mar 8, 2017

pjcozzi commented Mar 10, 2017


		Another limitation of the standard batch table is the difficulty in expressing metadata hierarchies.

		For example consider a tile that represents a city block. The block itself contains metadata, the individual buildings contain metadata, and the building components (doors, walls, roofs) contain metadata.


		### Notes

		* In some cases a feature may have multiple ancestors of the same class, or ancestors of different classes with the same property names. It is up to the implemantation to decide how to handle overloaded properties.

		```

		* The batch table hierarchy is self-contained within the tile. It is not possible to form metadata hierarchy across different tiles in the tileset.

Batch Table Hierarchy #171

Batch Table Hierarchy #171

Conversation

lilleyse commented Jan 18, 2017 • edited Loading

lilleyse commented Jan 19, 2017

pjcozzi commented Jan 21, 2017

pjcozzi commented Jan 21, 2017

pjcozzi commented Jan 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Jan 21, 2017

pjcozzi commented Jan 21, 2017

pjcozzi commented Jan 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Jan 21, 2017

pjcozzi commented Jan 21, 2017

pjcozzi commented Jan 21, 2017

pjcozzi commented Jan 21, 2017

lilleyse commented Jan 23, 2017

arneschilling commented Feb 1, 2017

lilleyse commented Feb 1, 2017

pjcozzi commented Feb 5, 2017

pmconne commented Feb 9, 2017

pmconne commented Feb 10, 2017

lilleyse commented Mar 2, 2017

Choose a reason for hiding this comment

lilleyse Mar 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Mar 8, 2017

pjcozzi commented Mar 10, 2017

lilleyse commented Jan 18, 2017 •

edited

Loading

lilleyse Mar 9, 2017 •

edited

Loading