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 development that supports both offline and online input. #28

Merged
merged 5 commits into from
Oct 11, 2024

Conversation

periakiva
Copy link

What does this PR do?

Fixes #<issue_number>

Before submitting

  • Did you make sure title is self-explanatory and the description concisely explains the PR?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you list all the breaking changes introduced by this pull request?
  • Did you test your PR locally with pytest command?
  • Did you run pre-commit hooks with pre-commit run -a command?

Did you have fun?

Make sure you had fun coding 🙃

@periakiva periakiva marked this pull request as draft October 8, 2024 20:18
@periakiva
Copy link
Author

@Purg

tcn_hpl/data/test.mscoco.json Outdated Show resolved Hide resolved


class TCNDataset(Dataset):
def __init__(self, kwcoco_path: str, sample_rate: int, window_size: int):
Copy link
Member

Choose a reason for hiding this comment

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

I was thinking that this dataset could take three paths as input: for object detection COCO, for pose COCO, and for activity truth COCO. If this is one combined file, we will still have to create this file outside of usage of this dataset whenever we change our detection or pose sources. I was thinking that if they are separate inputs, we could more easily configure mixing and matching for training experiments. Thoughts?

Copy link
Author

@periakiva periakiva Oct 9, 2024

Choose a reason for hiding this comment

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

The current offline pipeline works sequentially: we generate a detection kwcoco and input it to the pose generation script to populate it further with pose annotations. The script I wrote above takes the detection+pose combined kwcoco and populates it with activity GT. Changing to 3 different coco files would mean that we need to change the way we save our feature specific (i.e. pose, detection, activity) kwcocos as well, and regenerate them before continuing.

For offline training there shouldn't be a change of data sources or kwcoco generation pipeline. Outside offline training , I currently set up a "collect_input" function outside the dataset class, which should take care of data standardization for either offline or online data inputs.

Copy link
Member

@Purg Purg Oct 9, 2024

Choose a reason for hiding this comment

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

My thought was along the lines that one thing we are achieving here is removing the "bundle" meta-files that were a step in between processing detections/pose/activity-truth combinations. As this currently stands, we are reduced in what we are preprocessing, but still have to do some preprocessing to create this combined COCO file. Going forward, I was assuming we would be changing our detection and pose sources (yolov7 vs. yolov8 vs. yolov11, mmpose vs. yolov11 pose, etc.), in which case being able to just point the dataset at which results version to use for iterating would be easier than having to remember to create a preprocessed unified COCO file. Is that conceptualization off the mark?

Copy link
Author

Choose a reason for hiding this comment

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

it is not off the mark, though it depends on how the current code will be changed. If you want to simply take the new pose/detection models and replace them in the current code, then the pipeline stays the same (i.e. sequential input-output of kwcoco files and obtaining a unified kwcoco). It is probably possible to just create a new kwcoco file for each task (instead of adding annotations) easily enough, but we now generate more kwcoco files without improving or adding functionality.

The way I see it, there isnt anything inherently different in creating 3 kwcoco files versus 1 kwcoco file, since this is only being used in the offline system (since the online system gets its own logic written in the "collect_inputs" function). Also - with 3 kwcoco files, you will need to cross reference pose, detections, and activity GT to the same images, and will also need to ensure that frames have the same ids in all those kwcocos.

Copy link
Member

Choose a reason for hiding this comment

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

Separate question: are the individual detection / pose / activity classification COCO formats (all merged into this file, seemingly) following any standards? If so, could you remind me what/where those standards are? Last I thought I heard, the activity classification standard was one that we made up. If that is true, is there a better activity classification standard to use? I would almost thing that this is basically an image classification, so would it be better to follow that established format?

Copy link
Author

Choose a reason for hiding this comment

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

the training pipeline is setup as a classification problem - yes. I am not sure if I understand what you mean by the individual annotation coco format standards? If you mean in terms of metadata and category information - it is set up as a mix of pose and detection, without acitivity classification metadata (i.e. dict with id and name of activity classification categories) baked into the metadata. We use a separate config file (i.e. /home/local/KHQ/peri.akiva/projects/angel_system/config/activity_labels/medical/r18.yaml)

tcn_hpl/data/add_gt_to_kwcoco.py Outdated Show resolved Hide resolved
tcn_hpl/data/add_gt_to_kwcoco.py Outdated Show resolved Hide resolved
tcn_hpl/data/add_gt_to_kwcoco.py Outdated Show resolved Hide resolved
tcn_hpl/data/add_gt_to_kwcoco.py Outdated Show resolved Hide resolved


class TCNDataset(Dataset):
def __init__(self, kwcoco_path: str, sample_rate: int, window_size: int):
Copy link
Member

@Purg Purg Oct 9, 2024

Choose a reason for hiding this comment

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

My thought was along the lines that one thing we are achieving here is removing the "bundle" meta-files that were a step in between processing detections/pose/activity-truth combinations. As this currently stands, we are reduced in what we are preprocessing, but still have to do some preprocessing to create this combined COCO file. Going forward, I was assuming we would be changing our detection and pose sources (yolov7 vs. yolov8 vs. yolov11, mmpose vs. yolov11 pose, etc.), in which case being able to just point the dataset at which results version to use for iterating would be easier than having to remember to create a preprocessed unified COCO file. Is that conceptualization off the mark?

tcn_hpl/data/tcn_dataset.py Outdated Show resolved Hide resolved
@Purg
Copy link
Member

Purg commented Oct 9, 2024

Fixup commit (with integrating rebase) removed COCO file and added some nitpick whitespace updates

@Purg
Copy link
Member

Purg commented Oct 9, 2024

@periakiva I've done some minor updates based on my few comments. I'm happy to merge this unless you have any issues.

@periakiva periakiva marked this pull request as ready for review October 11, 2024 14:40
@Purg Purg merged commit 46e792a into PTG-Kitware:main Oct 11, 2024
0 of 11 checks passed
@Purg Purg deleted the dev-dset branch October 11, 2024 15:08
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.

2 participants