Add raft builders and presets in prototypes #5043

NicolasHug · 2021-12-07T11:41:22Z

Towards #4644

This PR adds the raft_large() and raft_small() builders, following the new model prototype API.

cc @datumbox @bjuncek

facebook-github-bot · 2021-12-07T11:41:30Z

💊 CI failures summary and remediations

As of commit 6a89b6d (more details on the Dr. CI page):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

NicolasHug · 2021-12-07T11:43:07Z

torchvision/prototype/transforms/_presets.py

+        return img1, img2, flow, valid_flow_mask
+
+    def _pil_or_numpy_to_tensor(self, img1, img2, flow, valid_flow_mask):
+        if not isinstance(img1, Tensor):


I find it a bit confusing that we type the input as Tensor and actually handle the case where it's not a tensor. I saw that on the other presets so I did the same. I assume that this is only temporary to check these presets on the current transforms (which return PIL images), and that we will remove the conversions eventually?

Good point. It's also because the user is supposed to use these transforms during inference. At that point, you don't know if they chose to read the image with PIL or with TV's io. So here we support both.

This is also done because the reference scripts for other tasks only support PIL. BTW now that you added a prototype section for your model, you should add a support for it in your reference scripts on the other PR.

NicolasHug · 2021-12-07T11:45:32Z

torchvision/prototype/models/optical_flow/raft.py

+        url="",
+        transforms=RaftEval,
+        meta={
+            "recipe": "",
+            "epe": -1234,
+        },


I'll fill these up ASAP when I get the results and when #5027 and #5026 are merged.

@datumbox can the meta field support epe for one than one dataset?
I assume I can just have different keys like "sintel_train_epe", "sintel_test_epe", "kitti_train_epe", etc?

There is currently no schema enforcement on the meta so in theory you can add whatever you want. Having some common names across tasks would be useful for us when we tackled the documentation improvements (for example we currently use the followings: recipe, size, interpolation). For within the same task, I think it makes sense to introduce task-specific fields (for example: detection uses map, quantized models support backend etc).

Your use-case is quite unique; so far we never had a situation where we train a model with one dataset and provide validation stats for many. We also tend to give the stats on validation set, not on training set. Perhaps in your case, is worth providing a single metric under epe which gives the value on the same dataset(s) it was used for fitting (or eval?) and then have another field other_epe (better name required) that's going to be a dictionary with entries the various datasets and their metrics.

datumbox

Thanks for the PR @NicolasHug. I've added a few comments to discuss with you, happy to chat offline if that's easier and summarize afterwards.

datumbox · 2021-12-07T12:46:40Z

torchvision/prototype/models/optical_flow/raft.py

+        url="",
+        transforms=RaftEval,
+        meta={
+            "recipe": "",
+            "epe": -1234,
+        },


There is currently no schema enforcement on the meta so in theory you can add whatever you want. Having some common names across tasks would be useful for us when we tackled the documentation improvements (for example we currently use the followings: recipe, size, interpolation). For within the same task, I think it makes sense to introduce task-specific fields (for example: detection uses map, quantized models support backend etc).

Your use-case is quite unique; so far we never had a situation where we train a model with one dataset and provide validation stats for many. We also tend to give the stats on validation set, not on training set. Perhaps in your case, is worth providing a single metric under epe which gives the value on the same dataset(s) it was used for fitting (or eval?) and then have another field other_epe (better name required) that's going to be a dictionary with entries the various datasets and their metrics.

torchvision/prototype/models/optical_flow/raft.py

datumbox · 2021-12-07T13:03:12Z

torchvision/prototype/transforms/_presets.py

+        return img1, img2, flow, valid_flow_mask
+
+    def _pil_or_numpy_to_tensor(self, img1, img2, flow, valid_flow_mask):
+        if not isinstance(img1, Tensor):


Good point. It's also because the user is supposed to use these transforms during inference. At that point, you don't know if they chose to read the image with PIL or with TV's io. So here we support both.

This is also done because the reference scripts for other tasks only support PIL. BTW now that you added a prototype section for your model, you should add a support for it in your reference scripts on the other PR.

torchvision/prototype/transforms/_presets.py

Co-authored-by: Vasilis Vryniotis <[email protected]>

…roto

datumbox

LGTM, only couple of minor nits and you are ready to merge on green CI.

torchvision/prototype/models/optical_flow/raft.py

torchvision/prototype/transforms/_presets.py

datumbox · 2021-12-07T18:24:59Z

@NicolasHug I just realized there are a few other things that we need to add in this PR, just because you introduce a new family of models (optical flow).

Add models.optical_flow in all places where we do:

vision/test/test_prototype_models.py

Lines 72 to 79 in 588e9b5

    
           @pytest.mark.parametrize( 
        
               "model_fn", 
        
               TM.get_models_from_module(models) 
        
               + TM.get_models_from_module(models.detection) 
        
               + TM.get_models_from_module(models.quantization) 
        
               + TM.get_models_from_module(models.segmentation) 
        
               + TM.get_models_from_module(models.video), 
        
           )

Add a test_raft method similar to the detection (not sure why we call it test_raft on the test_models.py file instead of test_optical_flow_model but the name should match between prototypes and non-prototypes)

vision/test/test_prototype_models.py

Lines 106 to 110 in 588e9b5

    
           @pytest.mark.parametrize("model_fn", TM.get_models_from_module(models.segmentation)) 
        
           @pytest.mark.parametrize("dev", cpu_and_gpu()) 
        
           @run_if_test_with_prototype 
        
           def test_segmentation_model(model_fn, dev): 
        
               TM.test_segmentation_model(model_fn, dev)

Add default params for "optical_flow" models on the following test (not sure what parameter is right for you here, but the test expects you to have it defined the moment you put weights):

vision/test/test_prototype_models.py

Lines 130 to 137 in 588e9b5

    
           def test_old_vs_new_factory(model_fn, dev): 
        
               defaults = { 
        
                   "models": { 
        
                       "input_shape": (1, 3, 224, 224), 
        
                   }, 
        
                   "detection": { 
        
                       "input_shape": (3, 300, 300), 
        
                   },

test/test_prototype_models.py

Co-authored-by: Vasilis Vryniotis <[email protected]>

NicolasHug · 2021-12-08T12:31:51Z

Thanks a lot for the reviews! I'll merge now, and will update these bits as we publish the pretrained weights - PR coming later today.

Summary: Co-authored-by: Vasilis Vryniotis <[email protected]> Reviewed By: NicolasHug Differential Revision: D32950935 fbshipit-source-id: b936d2879243f7d5b8e05f329ebdfd074f5af05b

Add raft builders and presets in prototypes

a1d5c24

pytorch-probot bot added the ciflow/default label Dec 7, 2021

facebook-github-bot added the cla signed label Dec 7, 2021

NicolasHug added module: models prototype and removed cla signed labels Dec 7, 2021

NicolasHug commented Dec 7, 2021

View reviewed changes

facebook-github-bot added the cla signed label Dec 7, 2021

Switch import

b436fc0

datumbox reviewed Dec 7, 2021

View reviewed changes

NicolasHug and others added 7 commits December 7, 2021 13:23

Merge branch 'main' of github.com:pytorch/vision into raft_proto

91e4084

Update torchvision/prototype/transforms/_presets.py

1f58b77

Co-authored-by: Vasilis Vryniotis <[email protected]>

Merge branch 'raft_proto' of github.com:NicolasHug/vision into raft_p…

d958643

…roto

Address comments

c8d65b6

Comment out unsued imports

17522f4

Comment out unsued imports

1dc06ba

typo flow -> valid_flow_mask

48674ba

datumbox approved these changes Dec 7, 2021

View reviewed changes

torchvision/prototype/models/optical_flow/raft.py Outdated Show resolved Hide resolved

torchvision/prototype/models/optical_flow/raft.py Outdated Show resolved Hide resolved

torchvision/prototype/transforms/_presets.py Show resolved Hide resolved

NicolasHug added 2 commits December 7, 2021 19:28

Merge branch 'main' of github.com:pytorch/vision into raft_proto

739924c

Add tests

68d1f41

NicolasHug commented Dec 7, 2021

View reviewed changes

test/test_prototype_models.py Show resolved Hide resolved

datumbox reviewed Dec 8, 2021

View reviewed changes

test/test_prototype_models.py Outdated Show resolved Hide resolved

NicolasHug and others added 2 commits December 8, 2021 09:06

Update test/test_prototype_models.py

814c20c

Co-authored-by: Vasilis Vryniotis <[email protected]>

Merge branch 'main' into raft_proto

aa66031

NicolasHug mentioned this pull request Dec 8, 2021

RAFT model and training reference #4644

Closed

12 tasks

Merge branch 'main' into raft_proto

6a89b6d

NicolasHug merged commit 5dc1e20 into pytorch:main Dec 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add raft builders and presets in prototypes #5043

Add raft builders and presets in prototypes #5043

NicolasHug commented Dec 7, 2021 •

edited by pytorch-probot bot

Loading

facebook-github-bot commented Dec 7, 2021 •

edited

Loading

NicolasHug Dec 7, 2021

datumbox Dec 7, 2021

NicolasHug Dec 7, 2021

datumbox Dec 7, 2021

datumbox left a comment

datumbox Dec 7, 2021

datumbox Dec 7, 2021

datumbox left a comment

datumbox commented Dec 7, 2021

NicolasHug commented Dec 8, 2021

Add raft builders and presets in prototypes #5043

Add raft builders and presets in prototypes #5043

Conversation

NicolasHug commented Dec 7, 2021 • edited by pytorch-probot bot Loading

facebook-github-bot commented Dec 7, 2021 • edited Loading

💊 CI failures summary and remediations

NicolasHug Dec 7, 2021

Choose a reason for hiding this comment

datumbox Dec 7, 2021

Choose a reason for hiding this comment

NicolasHug Dec 7, 2021

Choose a reason for hiding this comment

datumbox Dec 7, 2021

Choose a reason for hiding this comment

datumbox left a comment

Choose a reason for hiding this comment

datumbox Dec 7, 2021

Choose a reason for hiding this comment

datumbox Dec 7, 2021

Choose a reason for hiding this comment

datumbox left a comment

Choose a reason for hiding this comment

datumbox commented Dec 7, 2021

NicolasHug commented Dec 8, 2021

NicolasHug commented Dec 7, 2021 •

edited by pytorch-probot bot

Loading

facebook-github-bot commented Dec 7, 2021 •

edited

Loading