Object detection example #55

AngranLi · 2021-05-31T22:35:34Z

This PR consists of 3 parts:

The change of xt-training itself to make it work with the dictionary label yused in object detection.
The new metric of average precision, which is based on the code in https://github.com/rafaelpadilla/Object-Detection-Metrics.
The example in Jupyter Notebook. The structure and description is based on Jack's Image Classification example.

Change type:

Bug fix
Feature
Documentation

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code and added docstrings to all exposed functions and classes
I have made corresponding changes to the documentation
Any relevant changes to third party licenses have been updated in the README

jwmandeville

Looks good and I tested and it works on my system as well.

A few suggestions:

Add a link to the dataset download page: https://www.cis.upenn.edu/~jshi/ped_html/
Change the dataset paths and checkpoint paths to variables
Change the paths to something generic like path/to/dataset/PennFudanPed instead of your PC's location

AngranLi · 2021-06-02T20:58:02Z

Looks good and I tested and it works on my system as well.

A few suggestions:

Add a link to the dataset download page: https://www.cis.upenn.edu/~jshi/ped_html/

Change the dataset paths and checkpoint paths to variables

Change the paths to something generic like path/to/dataset/PennFudanPed instead of your PC's location

Thanks Jack! Just made the changes.

timesler · 2021-06-04T22:01:31Z

xt_training/runner.py

+                        y_tmp = []
+                        for y_i in y:
+                            if isinstance(y_i, dict):
+                                y_i = {k: v.to(device) for k, v in y_i.items()}
+                            else:
+                                y_i.to(device)
+                            y_tmp.append(y_i)
+                        y = y_tmp


Can we add a comment describing what is happening here and what the use case is?

Comment added.

timesler · 2021-06-04T22:05:46Z

xt_training/runner.py

+                try:
+                    y_pred = model(x)
+                    loss_batch = loss_fn(y_pred, y)
+                    # The output of Object Detection is a tuple of (loss, y_pred), and when 
+                    # not in training mode (model.training==False), loss is an empty dictionary.
+                    if isinstance(y_pred, tuple):
+                        y_pred = y_pred[-1]
+                except ValueError:
+                    # Object Detection training process requires (x, y) as input.
+                    loss, y_pred = model(x, y)
+                    # During training, y_pred is an empty list with len==0.
+                    # Assign a length to it to make the PooledMean metrics work.
+                    y_pred = [None]
+                    # The Object Detection model already calculated the loss, 
+                    # so loss_fn should be declared to work with it.
+                    loss_batch = loss_fn(y_pred=y_pred, y=loss)


I don't think we should rely on error handling to do non-erroneous control flow.

Also, in general, we shouldn't let the structure of a single object detection model impact the structure of our package so much. I think it would be better to modify the model so that it behaves normally and outputs only the predictions all the time and move the loss calculation to a separate loss function.

Yep, I made some modification and discarded the error handling.

timesler · 2021-06-04T22:07:35Z

xt_training/runner.py

                    elif isinstance(y, Iterable):
-                        y = [y_i.to(device) for y_i in y]


I'm pretty sure this will error for the data you are expecting. If y_i is a dict, then it won't have a to method

I think a better practice for object detection would be to put the to calls inside the forward method.

Thanks for the suggestion Tim. I wrote an ODInterface based on the SKInterface, and moved the to calls inside the forward method.

…rward() method.

…on the scikit learn wrappers.

AngranLi · 2021-06-23T22:35:33Z

@timesler The modifications I made:

modified runner.py so the error handling is discarded
add the object detection wrappers so I can move the to calls inside the forward method.

timesler

In general, I still think this departs too much from our standard workflow in xt-training, because we are trying to bend over backwards to reproduce a specific tutorial, or to allow the use of the torchvision object detection structure.

In essence, object detection should have no problems conforming to the normal process:

A dataset returns a tuple of x and y
The model forward function accepts a single argument (x) and always returns a single variable (y_pred)
Loss and metric functions accept y and y_pred and return scalar values

The way it's done in torchvision is different from the torchvision classification models but there's not good reason for that as far I know. I think we should build our example using a modified version of the model that doesn't do anything with loss inside the model itself.

timesler · 2021-07-16T21:34:48Z

xt_training/metrics.py

-        self.value_sum += self.latest_value.detach() * self.latest_num_samples
+        tmp = self.value_sum + self.latest_value.detach() * self.latest_num_samples
+        # self.value_sum += self.latest_value.detach() * self.latest_num_samples
+        self.value_sum = tmp


Is there any reason for this? I think it's equivalent

timesler · 2021-07-16T21:37:36Z

xt_training/metrics.py

+        """
+            cls: The class that is goint to calculate the average precision.
+            iouthreshold: The IOU threshold to determine whether a detection is true/false positive.
+            method: The interpolation method to calculate the average precision. Can choose between 
+                    every point interpolation or eleven point interpolation.
+        """


This doesn't follow our docstring style. I recommend installing the "Python Docstring Generator" extension in VSCode and configuring it to the correct style. That will make it really easy to add new docstrings with the correct format

timesler · 2021-07-16T21:40:34Z

xt_training/od_lib/BoundingBox.py

+        return newBoundingBox
+
+
+class BoundingBoxes:


If this is intended to be imported by users, we should add a docstring. If not, it should probably be named _BoundingBoxes to indicate it's private

AngranLi added 12 commits April 20, 2021 16:15

Created notebooks for xt-training examples.

1f4c7f9

Merge branch 'master' into examples

62ffdea

Merge branch 'master' into examples

771b651

Merge branch 'master' into examples

6b77e46

Merge with master

7598599

Merge with master.

1592253

Make runner.py work for both Object Detection model and other models.

1e660f7

Moved the example notebook to right place.

2ce20ec

Merge branch 'master' into object_detection

0bf5d3d

Implemented the new metric of Average Precision.

75e91f9

Improved the descriptions and comments for the Object Detection example.

205ee8f

Merge branch 'master' into object_detection

e27abc5

AngranLi requested review from jwmandeville, simonrmonk and timesler May 31, 2021 22:35

jwmandeville previously approved these changes Jun 2, 2021

View reviewed changes

AngranLi added 2 commits June 2, 2021 13:50

Minor change for pip package installation.

8bfd6ec

Changed the paths of my PC to more sensible representation.

2520a89

AngranLi dismissed jwmandeville’s stale review via 2520a89 June 2, 2021 20:57

jwmandeville previously approved these changes Jun 3, 2021

View reviewed changes

timesler suggested changes Jun 4, 2021

View reviewed changes

Added comment.

7c993a9

AngranLi dismissed jwmandeville’s stale review via 7c993a9 June 21, 2021 20:31

AngranLi added 4 commits June 23, 2021 15:09

Discard error handling, and move the to(device) process inside the fo…

6de382a

…rward() method.

Wrappers for handling object detection tasks with xt-training. Based …

de66227

…on the scikit learn wrappers.

Wrappers for handling object detection tasks with xt-training. Based …

50c0fce

…on the scikit learn wrappers.

Modification according to new xt-training runner script.

fdb9e26

AngranLi requested a review from timesler June 23, 2021 22:32

timesler suggested changes Jul 16, 2021

View reviewed changes

		elif isinstance(y, Iterable):
		y = [y_i.to(device) for y_i in y]

Object detection example #55

Are you sure you want to change the base?

Object detection example #55

Uh oh!

Conversation

AngranLi commented May 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jwmandeville left a comment

Choose a reason for hiding this comment

Uh oh!

AngranLi commented Jun 2, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AngranLi commented Jun 23, 2021

Uh oh!

timesler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AngranLi commented May 31, 2021 •

edited

Loading