-
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
[discussion] [wip] Move/adopt to torchvision frequent utils from detectron2 (and potentially other frameworks) #7070
Comments
Okay, let's discuss each of these.
Rotated operations need additional support as well as Transforms API etc. See #2761 It will also need to be compatible with new Transforms API etc.
torchvision has few colormap utils in torchvision. Please see https://github.com/pytorch/vision/blob/main/torchvision/utils.py#L486
Again torchvision has few visualization utils https://github.com/pytorch/vision/blob/main/torchvision/utils.py#L13
I have no idea. cc @vfdev-5 @pmeier
Torchvision already have focal loss. https://github.com/pytorch/vision/blob/main/torchvision/ops/focal_loss.py Let's discuss a bit more so we know requirements clearly. :) |
Re exif: I have no knowledge about that, but the function inside detectron two is over 2 years old and links to a now closed Pillow issue. According to the comments in the thread there, the patch landed in |
On About exif: also, are libjpg/libpng supporting parsing EXIF attributes from image files? or maybe can be done with a very simple python parser? I guess pillow is needed only for reading these attributes here. So without pillow, some sort of EXIF parser is needed. OpenCV also has some information about exif parsing in their changelog: https://github.com/opencv/opencv/wiki/ChangeLog#version452. This may be more related to I/O component. about focal loss: can we ask fvcore authors if their focal loss matches exactly the focal loss in torchvision? currently detectron2 uses some fvcore stuffs. I guess one could check all functions from fvcore used by detectron2 and check their usefulness for inclusion in core About paste_masks_in_image: why then small difference in API and why doesn't detectron2 use the torchvision's function if they are identical? about rotated boxes ops: converging on object-oriented structures can take a long time (i think transforms/prototype v2 is already lasting for 1 year? 2 years?), but having simple purely functional functions (accepting pure tensors with well-described format of docs) should require less of design and can even happen before full support of transforms - this would allow using these ops in other frameworks. other frameworks have their own modeling for "structures" / transforms anyway. but importing those transforms on "ops" level is also a good idea IMO. In general, my motivation for this discussion opening is that deduplicating common/frequently-used/useful well-contained/small-api idioms from other libraries will help them reduce the API surface, and these idioms would be useful for all downstream libraries and users and reduce the amount of boiler-plate. And that a systematic discussion with those library authors would be fruitful, they would also have interesting things to say which idioms/functions (especially well-contained and small API-surface) turned out to be well-designed and would be good to include in core for reusability Above I just scratched some utils that might be good to have in core while browsing the current detectron2 codebase, so the list is quite arbitrary. So this is not in stone, I just propose to look at those codebases with a fresh eye and ask whether some utils have become standard enough |
@oke-aditya for other mask ops/utils, I had posted some my ideas in #4415 |
🚀 The feature
I propose to open the discussion and collect in this issue some discrepancies or duplicate functionalities I found between detectron2 and torchvision.
detecton2 / mmdet / kornia are now quite old and established, so one can discuss of adopting well-designed / widely-used util functions and Modules from these frameworks to torchvision, so that there's less reimplementation and more established idioms.
Motivation, pitch
N/A
Alternatives
No response
Additional context
No response
The text was updated successfully, but these errors were encountered: