Separate coco from map #2884
Separate coco from map #2884
Conversation
Isalia20
requested review from
SkafteNicki,
Borda,
justusschock and
stancld
as code owners
|
Huh weird, I thought pre commit checks covered the mypy checks as well. Will fix them |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2884 +/- ##
=======================================
- Coverage 69% 69% -0%
=======================================
Files 346 333 -13
Lines 19146 18988 -158
=======================================
- Hits 13233 13106 -127
+ Misses 5913 5882 -31 |
|
I'll wait to get some comments on this first and then proceed with functional part of mAP |
apology for the delay, I'll have a look at it next week :) |
| >>> # Example files can be found at | ||
| >>> # https://github.com/cocodataset/cocoapi/tree/master/results | ||
| >>> from torchmetrics.detection import MeanAveragePrecision | ||
| >>> preds, target = MeanAveragePrecision.coco_to_tm( |
There was a problem hiding this comment.
| >>> preds, target = MeanAveragePrecision.coco_to_tm( | |
| >>> preds, target = MeanAveragePrecision().coco_to_tm( |
| ] | ||
|
|
||
| def __init__( | ||
| self, | ||
| box_format: Literal["xyxy", "xywh", "cxcywh"] = "xyxy", | ||
| iou_type: Union[Literal["bbox", "segm"], tuple[Literal["bbox", "segm"], ...]] = "bbox", | ||
| iou_type: Union[Literal["bbox", "segm"], tuple[str]] = "bbox", |
There was a problem hiding this comment.
why did the typing change here?
| def _load_coco_backend_tools(backend: Literal["pycocotools", "faster_coco_eval"]) -> tuple[object, object, ModuleType]: | ||
| """Load the backend tools for the given backend.""" | ||
| if backend == "pycocotools": | ||
| if not _PYCOCOTOOLS_AVAILABLE: | ||
| raise ModuleNotFoundError( | ||
| "Backend `pycocotools` in metric `MeanAveragePrecision` metric requires that `pycocotools` is" | ||
| " installed. Please install with `pip install pycocotools` or `pip install torchmetrics[detection]`" | ||
| ) | ||
| import pycocotools.mask as mask_utils | ||
| from pycocotools.coco import COCO | ||
| from pycocotools.cocoeval import COCOeval | ||
|
|
||
| return COCO, COCOeval, mask_utils | ||
|
|
||
| if not _FASTER_COCO_EVAL_AVAILABLE: | ||
| raise ModuleNotFoundError( | ||
| "Backend `faster_coco_eval` in metric `MeanAveragePrecision` metric requires that `faster-coco-eval` is" | ||
| " installed. Please install with `pip install faster-coco-eval`." | ||
| ) | ||
| from faster_coco_eval import COCO | ||
| from faster_coco_eval import COCOeval_faster as COCOeval | ||
| from faster_coco_eval.core import mask as mask_utils | ||
|
|
||
| return COCO, COCOeval, mask_utils |
There was a problem hiding this comment.
please move to the src/torchmetrics/detection/helpers.py file. While we do not have any strict rules for what goes to the general utilities folder, this is very much something that is unique to the detection domain and therefore should be kept there
| self.iou_type, | ||
| ) | ||
|
|
||
| def coco_to_tm( |
There was a problem hiding this comment.
would it be possible to make this a classmethod so the user can convert without initializing the class?
alternatively we should get rid of iou_type and backend arguments because they are already set in the class
|
Ah sorry, just saw this. I'll resolve the comments this week |
Perfect, thank you |
What does this PR do?
Part of the fix of #2877. This PR decouples the coco backend from the mAP metric. This is part of the fix of 2877 issue because for me to implement functional mAP I would have to duplicate a lot of code unless this class was created. This class will help to duplicate as little code as possible while implementing functional version of mAP. It also makes mAP less heavy with coco stuff and abstracts it away as a coco backend private instance
Before submitting
Did you have fun? yes
Make sure you had fun coding 🙃
📚 Documentation preview 📚: https://torchmetrics--2884.org.readthedocs.build/en/2884/