-
Notifications
You must be signed in to change notification settings - Fork 133
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
scannet dataset can pick the number of classes to use and referring e… #388
Conversation
…xpressions respect this choice
src/home_robot/home_robot/datasets/scannet/data/load_scannet_data.py
Outdated
Show resolved
Hide resolved
@@ -102,7 +105,7 @@ def load_referit3d_data( | |||
n_original = len(referit_data) | |||
referit_data = referit_data[referit_data["mentions_target_class"]] | |||
referit_data.reset_index(drop=True, inplace=True) | |||
print( | |||
logger.info( |
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.
I like this, we should switch to using logger more
@@ -1016,7 +1016,8 @@ | |||
"cd case", | |||
"closet rod", | |||
"coffee kettle", | |||
"wardrobe ", | |||
# "wardrobe ", |
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.
??
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.
Somehow the original scannet category has this trailing whitespace. It screws up some code that uses string-based matching :/
@@ -5602,7 +5603,7 @@ | |||
"cd case", | |||
"closet rod", | |||
"coffee kettle", | |||
"wardrobe ", |
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.
is this space important?
# labels_pd.loc[labels_pd.raw_category == 'stick', ['category']] = 'object' | ||
# labels_pd.loc[labels_pd.category == 'wardrobe ', ['category']] = 'wardrobe' | ||
# CLASS_ID_TO_NAME = dict(zip(labels_pd['id'], labels_pd['category'])) | ||
CLASS_ID_TO_NAME = { |
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.
Should we point to a specific scannet version to make sure this stays in sync? Not that i expect it to change or anything
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.
Thing that changes seems to be the number of classes, and whether those classes come from the scannet annotators or else MP40 or NYU40 classes. That mapping is in this csv, and we might want to restrict our attention to some subset of the full scannet categories (that's what this constants file is supposed to do).
Eventually we probably want to pin the evaluation config for the dataset (e.g. frame skip, image size, etc) but I think we can do that in configs
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.
lgtm
Issue:
Fix:
This PR gets the classes for referring expressions by cross-referencing the target object id of the expression with the class names used by the dataset.
Added ability to choose class naming + subsets in the dataset
Misc:
Some additional bugfixes for hydra configs in scannet_offline_eval to keep those up-to-date with InstanceMemory API