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.

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

Dataset Refactor #1

Closed
wants to merge 24 commits into from
Closed

Dataset Refactor #1

wants to merge 24 commits into from

Conversation

deepakgopinath
Copy link
Collaborator

  1. Base class for Dataset (implements all getters for different caches [ video frame, optic flow, segmentation, gaze data])
  2. Three Derived classes. [Gaze, Att_Labels, Pairwise Gaze]
  3. Cleaned up args file (potentially more clean up to happen later on)
  4. init.py in all folders.
  5. Simple dataset test script.

Copy link

@SimonStent-TRI SimonStent-TRI left a comment

Choose a reason for hiding this comment

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

Just some minor aesthetic suggestions -

  • Suggest doing a global search and replace for CognitiveHeatMap -> CHM.
  • # comments before the line of code (vs. on same line after) reads better?
  • .pyc files should not be committed?


def __getitem__(self, idx):
"""
Required getitem() for PyTorch gaze dataset.

Choose a reason for hiding this comment

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

PyTorch gaze dataset?

assert (
self.att_awareness_labels_csv_path is not None
), "Please provide the full path to the awareness labels csv file"
df = pd.read_csv(

Choose a reason for hiding this comment

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

weird to split this over two lines. Prefer the style of # comment prior to code, and code on one line?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, Will fix this. Prior to code is better.

help="filtering used for downsampling optic flow side channel inp [max, avg, median]",
)

##########################

Choose a reason for hiding this comment

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

missing comment?


pairwise_dataset = CognitiveHeatMapPairwiseGazeDataset("train", params_dict)
pairwise_data_dict = pairwise_dataset[0] # contains data_t, data_tp1
import IPython

Choose a reason for hiding this comment

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

this is called dataset_test, but it's not running a test? maybe it's better to have this as a notebook to illustrate the data dictionary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I will do that towards the end. This was more like a temporary script to test stuff on AWS.

@@ -0,0 +1,125 @@
# Copyright 2021 Toyota Research Institute. All rights reserved.

# This file details the field names and some notations used in datasets' getitems for CHM training

Choose a reason for hiding this comment

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

seems like it would be a lot cleaner to have most of this within a single dictionary of constants, not sure it's worth the effort though?

dest="no_save_model",
action="store_true",
default=False,
help="when set, model saving does not happen. ",
Copy link
Collaborator

Choose a reason for hiding this comment

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

W

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.

None yet

3 participants