[esp/scene] initial support for Gibson semantic scene #393

msbaines · 2019-12-18T22:10:03Z

This change include a Gibson semantic scene generator and parser.

Motivation and Context

Required to support Gibson semantic mesh data.

How Has This Been Tested

Wrote a simple test.

Types of changes

Docs change / refactoring / dependency upgrade
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have completed my CLA (see CONTRIBUTING)
I have added tests to cover my changes.
All new and existing tests passed.

This change include a Gibson semantic scene generator and parser.

codecov-io · 2019-12-19T00:44:27Z

Codecov Report

Merging #393 into master will decrease coverage by 1.33%.
The diff coverage is 77.15%.

@@            Coverage Diff             @@
##           master     #393      +/-   ##
==========================================
- Coverage   57.16%   55.82%   -1.34%     
==========================================
  Files         156      170      +14     
  Lines        7122     7918     +796     
  Branches        0       82      +82     
==========================================
+ Hits         4071     4420     +349     
- Misses       3051     3498     +447

Flag	Coverage Δ
#CPP	`52.35% <72.5%> (?)`
#JavaScript	`5.53% <ø> (?)`
#Python	`77.42% <100%> (?)`

Impacted Files	Coverage Δ
src/esp/assets/MeshData.h	`100% <ø> (ø)`
src/esp/scene/SemanticScene.h	`14.28% <ø> (ø)`	⬆️
src/esp/gfx/Simulator.h	`100% <ø> (ø)`	⬆️
src/esp/assets/MeshMetaData.h	`81.81% <ø> (-4.39%)`	⬇️
habitat_sim/nav/__init__.py	`100% <ø> (ø)`	⬆️
src/esp/assets/GenericInstanceMeshData.h	`0% <ø> (ø)`	⬆️
src/esp/geo/geo.cpp	`34.28% <0%> (+4.28%)`	⬆️
src/esp/gfx/PTexMeshShader.cpp	`0% <0%> (ø)`	⬆️
src/esp/nav/PathFinder.h	`26.66% <0%> (-46.07%)`	⬇️
src/esp/gfx/Simulator.cpp	`63.46% <0%> (-2.99%)`	⬇️
... and 38 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3523aa...4a9e94a. Read the comment docs.

mosra · 2019-12-19T18:55:12Z

src/esp/scene/GibsonSemanticScene.cpp

+    const std::string& houseFilename,
+    SemanticScene& scene,
+    const quatf& worldRotation /* = quatf::Identity() */) {
+  if (!io::exists(houseFilename)) {


Cr::Utility::Directory::exists()? IIRC the io:: APIs are obsolete and should be gradually replaced with corrade's functionality.

mosra · 2019-12-19T18:56:38Z

src/esp/scene/GibsonSemanticScene.cpp

+#include <map>
+#include <string>
+
+#include <Corrade/Utility/String.h>


Unused include?

msbaines · 2019-12-20T16:34:34Z

ping

All feedback addressed.

mathfac

Looks great, some minor comments. Can we add example of command line how to run the tool.

src/esp/gfx/Simulator.cpp

mathfac · 2019-12-20T01:27:23Z

src/esp/scene/GibsonSemanticScene.cpp

+bool SemanticScene::loadGibsonHouse(
+    const std::string& houseFilename,
+    SemanticScene& scene,
+    const quatf& worldRotation /* = quatf::Identity() */) {


Do we want to uncomment default value? Looks reasonable.

The default arguments are only provided in the function declaration, not in the function definition.

mathfac · 2019-12-20T01:43:42Z

src/esp/scene/GibsonSemanticScene.cpp

+      scene.categories_.push_back(category);
+      object->category_ = std::move(category);
+    }
+    // TODO(msb) object->obb = ;


Let's add more extended comment about adding bounding boxes.

mathfac · 2019-12-20T19:01:19Z

tools/npz2scn.py

+def fixCoords(entry):
+    size = entry["size"].tolist()
+    size[1], size[2] = size[2], size[1]
+    entry["size"] = size


Looks like entry["size"] wasn't list, but now it is. Is that expected?

mathfac · 2019-12-20T19:02:27Z

tools/npz2scn.py

+
+# Convert ndarrays to python lists so that we can serialize.
+# Tranform coordinates by rotating Y-axis to Z-axis
+def fixCoords(entry):


If we know entry type we can add typing to the func, for example:
from typing import Any, Dict
def fix_coords(entry: Dict[str, Any]) -> None

mathfac · 2019-12-20T19:03:25Z

tools/npz2scn.py

+
+
+# Convert ndarrays to python lists so that we can serialize.
+# Tranform coordinates by rotating Y-axis to Z-axis


Suggested change

# Tranform coordinates by rotating Y-axis to Z-axis

# Transform coordinates by rotating Y-axis to Z-axis

Maybe, the comment should be:
# Transform coordinates by rotating Y-axis to -Z-axis

mathfac · 2019-12-20T19:19:29Z

tools/npz2scn.py

+
+# Convert ndarrays to python lists so that we can serialize.
+# Tranform coordinates by rotating Y-axis to Z-axis
+def fixCoords(entry):


Based on naming convention:

Suggested change

def fixCoords(entry):

def fix_coords(entry):

mathfac · 2019-12-20T19:20:56Z

tools/npz2scn.py

+
+
+def main():
+    parser = argparse.ArgumentParser(description="Extract object IDs from npz.")


Suggested change

parser = argparse.ArgumentParser(description="Extract object IDs from npz.")

parser = argparse.ArgumentParser(description="Extracts object IDs from npz. Used for loading 3dscenegraph.stanford.edu semantic data.")

apsdehal · 2019-12-20T20:04:33Z

tools/npz2scn.py

+    for object in objects.values():
+        fixCoords(object)


Use of keyword object, rename to obj maybe.

bigbike · 2019-12-20T20:38:54Z

src/esp/scene/GibsonSemanticScene.cpp

+    } else {
+      int nextCategoryIndex = scene.categories_.size();
+      categories[categoryName] = nextCategoryIndex++;
+      auto category = std::make_shared<GibsonObjectCategory>(nextCategoryIndex,


nextCategoryIndex has been increased by 1.
It is NOT equal to categories[categoryName] now. Make sure if this is really what you want?

msbaines · 2019-12-20T23:03:39Z

I've now addressed all feedback.

…is missing (#406) With current code state 3dscenegraph semantic annotation files (*.scn) won't load, as our semantic loading pipeline triggers only on *.house files. To enable functionality implemented in #393 and #374 added loading of Gibson Semantics scene if MP3D semantic is missing. To test semantic loading e2e added integration test that will run only when *.scn test data is available.

…oc-in-CI Build api doc in CI

…ch#393)

…is missing (facebookresearch#406) With current code state 3dscenegraph semantic annotation files (*.scn) won't load, as our semantic loading pipeline triggers only on *.house files. To enable functionality implemented in facebookresearch#393 and facebookresearch#374 added loading of Gibson Semantics scene if MP3D semantic is missing. To test semantic loading e2e added integration test that will run only when *.scn test data is available.

[esp/scene] initial support for Gibson semantic scene

c522a1c

This change include a Gibson semantic scene generator and parser.

msbaines requested review from msavva, mathfac and erikwijmans December 18, 2019 22:10

facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Dec 18, 2019

msbaines requested a review from mosra December 18, 2019 22:12

Add Gibson semantic scene loading support to Simulator.

dbaa26f

Extend test to increase code coverage.

d3523aa

mosra reviewed Dec 19, 2019

View reviewed changes

Mandeep Singh Baines added 3 commits December 19, 2019 15:20

Merge remote-tracking branch 'origin/master' into gibson-scene

d9e8c28

Address @mosra feedback

3932a6a

fix comment typo

7b67773

msbaines requested a review from bigbike December 20, 2019 03:56

mathfac approved these changes Dec 20, 2019

View reviewed changes

mathfac reviewed Dec 20, 2019

View reviewed changes

apsdehal reviewed Dec 20, 2019

View reviewed changes

Address @mathfac feedback

456b33f

bigbike reviewed Dec 20, 2019

View reviewed changes

Mandeep Singh Baines added 3 commits December 20, 2019 13:10

Address @mathfac feedback

4a126a3

Address @apsdehal feedback

2f513f0

Address @bigbike feedback

4a9e94a

msbaines merged commit 998f3f7 into master Dec 20, 2019

msbaines deleted the gibson-scene branch December 20, 2019 23:36

mathfac mentioned this pull request Dec 28, 2019

[esp/scene] Added loading of Gibson Semantics scene if MP3D semantic is missing, integration test #406

Merged

2 tasks

eundersander pushed a commit to eundersander/habitat-sim that referenced this pull request Aug 6, 2020

Merge pull request facebookresearch#393 from facebookresearch/build-d…

8249bce

…oc-in-CI Build api doc in CI

Ram81 pushed a commit to Ram81/habitat-web-sim that referenced this pull request Dec 10, 2020

[esp/scene] initial support for Gibson semantic scene (facebookresear…

896bd11

…ch#393)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[esp/scene] initial support for Gibson semantic scene #393

[esp/scene] initial support for Gibson semantic scene #393

msbaines commented Dec 18, 2019

codecov-io commented Dec 19, 2019 •

edited

Loading

mosra Dec 19, 2019

mosra Dec 19, 2019

msbaines commented Dec 20, 2019

mathfac left a comment

mathfac Dec 20, 2019

bigbike Dec 20, 2019

mathfac Dec 20, 2019

mathfac Dec 20, 2019

mathfac Dec 20, 2019 •

edited

Loading

mathfac Dec 20, 2019 •

edited

Loading

mathfac Dec 20, 2019

mathfac Dec 20, 2019

apsdehal Dec 20, 2019

bigbike Dec 20, 2019

msbaines commented Dec 20, 2019



		# Convert ndarrays to python lists so that we can serialize.
		# Tranform coordinates by rotating Y-axis to Z-axis

	# Tranform coordinates by rotating Y-axis to Z-axis
	# Transform coordinates by rotating Y-axis to Z-axis



		def main():
		parser = argparse.ArgumentParser(description="Extract object IDs from npz.")

[esp/scene] initial support for Gibson semantic scene #393

[esp/scene] initial support for Gibson semantic scene #393

Conversation

msbaines commented Dec 18, 2019

Motivation and Context

How Has This Been Tested

Types of changes

Checklist

codecov-io commented Dec 19, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msbaines commented Dec 20, 2019

mathfac left a comment

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

Choose a reason for hiding this comment

mathfac Dec 20, 2019 • edited Loading

Choose a reason for hiding this comment

mathfac Dec 20, 2019 • 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

Choose a reason for hiding this comment

msbaines commented Dec 20, 2019

codecov-io commented Dec 19, 2019 •

edited

Loading

mathfac Dec 20, 2019 •

edited

Loading

mathfac Dec 20, 2019 •

edited

Loading