-
Notifications
You must be signed in to change notification settings - Fork 7k
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
Add SSD in models #440
Comments
Hi, Thanks! I'm still figuring out the right balance between having things in torchvision and in external repos. I think everything that is quite generic and reusable should come here. One thing that needs to be improved is support for other data types than images (like bounding boxes). We've addressed that with the functional interface in some way, but we are still missing a good story on how to tie things together. What do you think? If you find time to start working on SSD, it might be good to list here your proposed action points so we can discuss. |
TODO
|
The thing I noticed while writing an initial version of Faster R-CNN in 2016 was that it requires a lot of code. This made me unsure if the About bounding box, I was mostly thinking about basic transforms, but other functionality (like intersection over union) might be worth considering. |
There is clearly code that is not part of the model but that is needed to use it. With SSD there is the box encoding/decoding procedure which is specific to this model, and that is quite heavy. There is the prior box management, the non-max suppression in the decoding... Do you think we could have a Something like:
I think that's reasonable. |
Hi guys, In ssd.pytorch I've tried to avoid modifying the current torchvision datasets (COCO, Pascal) and transformations as much as possible, but I think there would still need to be some slight modifications made to the current torchvision code to support detection, which I would be more than happy to help with, if we end up deciding on this route. That being said, I agree Francisco that most detection implementations out there are a little heavier and would potentially require a lot more to support (e.g. yolo, faster-rcnn), so I think it makes sense to consider whether or not a torchvision "detection module" could be made extensible to more than just SSD before we jump into it. |
@lemairecarl what are your thoughts on how to
Do you want to merge the current |
I think we'll see in the process if we need to break things into multiple files. I'll rely on the pull request comments. I want to start working on this next week. |
Yes. I'm still thinking on how to integrate the transforms in a way that it fits nicely with torchvision, while not requiring much boilerplate and being generic. |
We can inspire from tensorpack. For example, there is a proxy dataset that applies transformations on a dataset according to the indices: class XYTransformedDataset(Dataset):
def __init__(self, dataset, transformations, img_index=(0, 1), coords_index=(2, )):
self.ds = dataset
self.transformations = transformations
self.img_index = img_index
self.coords_index = coords_index
def __getitem__(self, index):
dp = self.ds[index] # For example, dp = (im, mask, polygons, labels)
output_dp = list(dp)
# HERE WE NEED TO PASS input image AS PARAMETER
params = self.transformations.get_params(dp[0])
# Transform images:
for idx in self.img_index:
output_dp[idx] = self.transformations(dp[idx], params)
# Transform coords:
for idx in self.coords_index:
output_dp[idx] = self.transformations.transform_coords(dp[idx], params)
return output_dp A base class for transformations: class BaseRandomTransformation:
def get_params(self):
return None
def __call__(self, img, params=None):
raise NotImplementedError()
def transform_coords(self, coords, params):
raise NotImplementedError() such that all other classes from torchvision derive from it. For example, class Compose(BaseRandomTransformation):
def __init__(self, transforms):
self.transforms = transforms
def __call__(self, img, params=None):
for t in self.transforms:
img = t(img, params=params)
return img
def transform_coords(self, coords, params):
for t, p in zip(self.transforms, params):
coords = t.transform_coords(coords, params=p)
return coords
def get_params(self, img):
return [t.get_params(img) for t in self.transforms] and another example: class RandomCrop(BaseRandomTransformation):
def __init__(self, size, padding=0):
# Same as in torchvision.RandomCrop
pass
def get_params(self, img):
return _get_params(img, self.size):
@staticmethod
def _get_params(img, output_size):
# Same as in torchvision.RandomCrop
w, h = img.size
th, tw = output_size
if w == tw and h == th:
return 0, 0, h, w
i = random.randint(0, h - th)
j = random.randint(0, w - tw)
return i, j, th, tw
def __call__(self, img, params=None):
if self.padding > 0:
img = F.pad(img, self.padding)
if params is None:
params = self._get_params(img, self.size)
i, j, h, w = params
return F.crop(img, i, j, h, w)
def transform_coords(self, coords, params):
i, j, h, w = params
return F.crop_coords(coords, i, j, h, w) Here, the problem is that some transformation parameters can not be generated without the input image. |
@fmassa any updates on this ? |
@vfdev-5 Yes, I have some proof-of-concept implementations. I'm holding on on making a PR yet because I want to see how well they fit the object detection framework I'm writing (I have Fast R-CNN, Faster R-CNN, and FPN working for training and evaluation, I'm now implementing Mask R-CNN). If I'm happy with how they mix with the framework, I'll be pushing them as is to torchvision. |
@fmassa that's super! |
It's not yet open-source, but it will be open-sourced. Stay tuned! |
@vfdev-5 One can find an SSD implementation in the fast.ai course lectures also. However, it is a bit hidden under the hood of author's wrappers. |
@devforfu yes, it is going to be similar to Detectron. |
@fmassa I've built a generic bounding box library for both 2D and 3D bounding boxes. I need to get some legal stuff taken care of before I can release it, but I believe it has everything you would need for object detection in general (including IoU computation). |
@varunagrawal we will be releasing in one week a library for object detection which will contain bounding box abstractions, and once it gets a bit more mature we might move it to torchvision. |
Hi @fmassa, was it released? |
@mattans yes, check it out in https://github.com/facebookresearch/maskrcnn-benchmark/ |
I'm closing this for now, since I have moved on to other projects. I might come back to it later. |
I look forward to this project going on, as I always love torchvision's elegant implementation.
|
Summary: Pull Request resolved: pytorch/kineto#440 Fixing windows build for `torchvision` and `kineto`. Differential Revision: D31295975 fbshipit-source-id: a2049218f46beb46bbaeb0a3b39d7633e695a799
Summary: Pull Request resolved: pytorch/kineto#440 Fixing windows build for `torchvision`. In `csrc/vision.cpp`, since `PyMODINIT_FUNC` depends on `Python.h` I added the same condition for `PyMODINIT_FUNC` as the one for `import <PyTorch.h>`. Differential Revision: D31488734 fbshipit-source-id: 0ca13c7d8de81f27eb63d3f7e54f8777128312c7
I've been working with PyTorch for several months, and with SSD for a few months. I'd like to add SSD to torchvision's "model zoo".
I will combine the good parts of https://github.com/amdegroot/ssd.pytorch and https://github.com/kuangliu/torchcv. Both implementation have some problems and refactoring will be needed to come to the level of refinement expected from torchvision.
I will begin in the coming weeks if there's no opposition.
@fmassa
The text was updated successfully, but these errors were encountered: