Add --backend and --use-v2 support for segmentation references #7743
Conversation
| # We need a custom pad transform here, since the padding we want to perform here is fundamentally | ||
| # different from the padding in `RandomCrop` if `pad_if_needed=True`. |
There was a problem hiding this comment.
If we at some point drop v1, we should re-evaluate if the custom scheme actually has an effect on the performance or if we can just RandomCrop. Until then, I agree we should keep the implementation as close as possible.
references/segmentation/presets.py
Outdated
|
|
||
| if use_v2: | ||
| dtype = { | ||
| (datapoints.Image if backend == "datapoint" else torch.Tensor): torch.float, |
There was a problem hiding this comment.
This will convert the dtype, but not the value range. We need to also have a ConvertDtype or its alias ConvertImageDtype here.
| transforms += [T.RandomResize(min_size=base_size, max_size=base_size)] | ||
|
|
||
| if backend == "pil": | ||
| # Note: we could just convert to pure tensors even in v2? |
There was a problem hiding this comment.
Depends on our decision in #7340. I would drop the comment here, because we'll easily forget about it.
| # elif "SLURM_PROCID" in os.environ: | ||
| # args.rank = int(os.environ["SLURM_PROCID"]) | ||
| # args.gpu = args.rank % torch.cuda.device_count() |
There was a problem hiding this comment.
?
| # elif "SLURM_PROCID" in os.environ: | |
| # args.rank = int(os.environ["SLURM_PROCID"]) | |
| # args.gpu = args.rank % torch.cuda.device_count() | |
| elif "SLURM_PROCID" in os.environ: | |
| args.rank = int(os.environ["SLURM_PROCID"]) | |
| args.gpu = args.rank % torch.cuda.device_count() |
There was a problem hiding this comment.
It's just to be able to run those scripts without invoking torchrun from the cluster (it's a bit of a mess). I'll remove it before merging
| import v2_extras | ||
| from torchvision.datasets import wrap_dataset_for_transforms_v2 | ||
|
|
||
| transforms = Compose([v2_extras.CocoDetectionToSegmentation(), transforms]) |
There was a problem hiding this comment.
We can pass CAT_LIST as arg to CocoDetectionToSegmentation to construct COCO_TO_VOC_LABEL_MAP instead of duplicating the data
There was a problem hiding this comment.
I would argue that this list is fixed and will never change. Thus, there is no point in passing it as a parameter. Meaning, if anything, I would change FilterAndRemapCocoCategories to also hardcode this.
No strong opinion, but I would not change anything here.
There was a problem hiding this comment.
Yeah, I agree this is unlikely to change. And as mention in the comment above, we could refactor both code-paths and unify some stuff, but I feel like it's best not to: we can keep the v2 path clean without being polluted with the mess accumulated over the years in the v1 path.
| import v2_extras | ||
| from torchvision.datasets import wrap_dataset_for_transforms_v2 | ||
|
|
||
| transforms = Compose([v2_extras.CocoDetectionToSegmentation(), transforms]) |
There was a problem hiding this comment.
I would argue that this list is fixed and will never change. Thus, there is no point in passing it as a parameter. Meaning, if anything, I would change FilterAndRemapCocoCategories to also hardcode this.
No strong opinion, but I would not change anything here.
|
Hey @NicolasHug! You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py |
…es (#7743) Reviewed By: matteobettini Differential Revision: D48642316 fbshipit-source-id: 7510049cae962545d263c824791567358d659d6d
No description provided.