Skip to content

Support undefined number of data points in the mesh.#2373

Merged
stephanwlee merged 21 commits into
tensorflow:masterfrom
podlipensky:tf-graphics-dyn
Jul 15, 2019
Merged

Support undefined number of data points in the mesh.#2373
stephanwlee merged 21 commits into
tensorflow:masterfrom
podlipensky:tf-graphics-dyn

Conversation

@podlipensky
Copy link
Copy Markdown
Contributor

Adds support of undefined (None) number of vertices/faces/colors in the mesh.

How to test:

  • Run updated unit tests.
  • Modify mesh_demo.py to provide different number of points at each run:
# Add extra vertex (1,2,3) to the mesh.
vertices_2 = np.concatenate((vertices, [[1, 2, 3]]))  
vertices_2 = np.expand_dims(vertices_2, 0)
...
for i in range(_MAX_STEPS):
    ver = vertices if i % 2 == 0 else vertices_2
    summary = sess.run(meshes_summary, feed_dict={
        vertices_tensor: ver,
        faces_tensor: faces,
        colors_tensor: colors,
        step: i,
    })

Run demo app and then launch tensorboard - observe mesh being rendered.

Also this PR changes the way we load data - from now on, every step is considered as unique data point and should be requested separately from the server (similar to image plugin). Please note that each mesh/step will produce 1 to 3 requests based on what data was passed into the summary (i.e. if there are no colors specified, then it will request only vertices and faces).

This PR depends on some TF-Graphics changes (I'll update it with new SHA once changes merged).

Copy link
Copy Markdown
Contributor

@stephanwlee stephanwlee left a comment

Choose a reason for hiding this comment

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

About the JavaScript reviews: I can send you a pull request if you think that helps.

Comment thread tensorboard/plugins/mesh/summary.py Outdated
Comment thread tensorboard/plugins/mesh/summary.py Outdated
Comment thread tensorboard/plugins/mesh/mesh_plugin.py
Comment thread tensorboard/plugins/mesh/mesh_plugin.py Outdated
Comment thread tensorboard/plugins/mesh/mesh_plugin_test.py Outdated
Comment thread tensorboard/plugins/mesh/tf_mesh_dashboard/mesh-loader.js Outdated
Comment thread tensorboard/plugins/mesh/tf_mesh_dashboard/mesh-loader.js Outdated
Comment thread tensorboard/plugins/mesh/tf_mesh_dashboard/mesh-loader.js Outdated
Comment thread tensorboard/plugins/mesh/tf_mesh_dashboard/mesh-loader.js Outdated
Comment thread tensorboard/plugins/mesh/tf_mesh_dashboard/mesh-loader.js Outdated
@stephanwlee stephanwlee requested a review from wchargin June 21, 2019 15:53
@stephanwlee
Copy link
Copy Markdown
Contributor

+reviewer: wchargin for Python readability.

Copy link
Copy Markdown
Contributor

@wchargin wchargin left a comment

Choose a reason for hiding this comment

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

review for python

Comment thread tensorboard/plugins/mesh/tf_mesh_dashboard/mesh-loader.js Outdated
Comment thread tensorboard/plugins/mesh/mesh_plugin.py Outdated
Comment thread tensorboard/plugins/mesh/mesh_plugin.py Outdated
Comment thread tensorboard/plugins/mesh/mesh_plugin.py Outdated
Comment thread tensorboard/plugins/mesh/mesh_plugin_test.py Outdated
Comment thread tensorboard/plugins/mesh/mesh_plugin_test.py Outdated
Comment thread tensorboard/plugins/mesh/plugin_data.proto Outdated
Comment thread tensorboard/plugins/mesh/plugin_data.proto Outdated
Comment thread tensorboard/plugins/mesh/summary.py Outdated
Comment thread tensorboard/plugins/mesh/test_utils.py Outdated
Comment thread tensorboard/plugins/mesh/plugin_data.proto Outdated
Comment thread tensorboard/plugins/mesh/plugin_data.proto Outdated
Comment thread tensorboard/plugins/mesh/summary.py Outdated
Comment thread tensorboard/plugins/mesh/mesh_plugin_test.py Outdated
Copy link
Copy Markdown
Contributor

@wchargin wchargin left a comment

Choose a reason for hiding this comment

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

Requesting changes to maintain data compatibility.

Comment thread tensorboard/plugins/mesh/metadata.py Outdated
Comment thread tensorboard/plugins/mesh/metadata.py Outdated
Comment thread tensorboard/plugins/mesh/metadata.py Outdated
Comment thread tensorboard/plugins/mesh/metadata.py Outdated
Comment thread tensorboard/plugins/mesh/plugin_data.proto
Comment thread tensorboard/plugins/mesh/metadata.py Outdated
Comment thread tensorboard/plugins/mesh/metadata_test.py Outdated
Comment thread tensorboard/plugins/mesh/metadata_test.py Outdated
Comment thread tensorboard/plugins/mesh/metadata_test.py
@podlipensky
Copy link
Copy Markdown
Contributor Author

FYI, updated SHA for TF Graphics-hosted files, so you can test the whole thing if needed. Please let me know if there is anything else is needed to merge the change.

Comment thread tensorboard/plugins/mesh/tf_mesh_dashboard/mesh-loader.js
Comment thread tensorboard/plugins/mesh/summary.py Outdated
Comment thread tensorboard/plugins/mesh/summary.py Outdated
Copy link
Copy Markdown
Contributor

@stephanwlee stephanwlee left a comment

Choose a reason for hiding this comment

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

FE and most changes LGTM. I will let William finish review the Python part.

Comment thread tensorboard/plugins/mesh/metadata.py Outdated
Comment thread tensorboard/plugins/mesh/plugin_data.proto Outdated
Comment thread tensorboard/plugins/mesh/metadata.py Outdated
Comment thread tensorboard/plugins/mesh/mesh_plugin.py Outdated
Comment thread tensorboard/plugins/mesh/mesh_plugin.py
Comment thread tensorboard/plugins/mesh/metadata.py Outdated
Comment thread tensorboard/plugins/mesh/http_api.md
Comment thread tensorboard/plugins/mesh/plugin_data.proto
Comment thread tensorboard/plugins/mesh/summary.py Outdated
Comment thread tensorboard/plugins/mesh/summary.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants