-
Notifications
You must be signed in to change notification settings - Fork 7.1k
Add pretrained weights on Chairs and Things for raft_large #5060
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
Conversation
💊 CI failures summary and remediationsAs of commit 57aff36 (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. |
@@ -19,6 +20,9 @@ | |||
) | |||
|
|||
|
|||
_MODELS_URLS = {"raft_large": "https://download.pytorch.org/models/raft_large_C_T_V2-1bb1363a.pth"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once PR is merged I will upload this to manifold
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: all current models use model_urls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @NicolasHug, I've added a few minor comments and nits. Let me know what you think.
Also there are a few more tests on the prototype where you should add models.optical_flow
, for example the test_schema_meta_validation
.
There you need to add the schema for optical flow:
vision/test/test_prototype_models.py
Lines 96 to 105 in 5dc1e20
def test_schema_meta_validation(model_fn): | |
classification_fields = ["size", "categories", "acc@1", "acc@5"] | |
defaults = { | |
"all": ["interpolation", "recipe"], | |
"models": classification_fields, | |
"detection": ["categories", "map"], | |
"quantization": classification_fields + ["backend", "quantization", "unquantized"], | |
"segmentation": ["categories", "mIoU", "acc"], | |
"video": classification_fields, | |
} |
In your case it's going to be empty, unless you add epe
or size
.
@@ -19,6 +20,9 @@ | |||
) | |||
|
|||
|
|||
_MODELS_URLS = {"raft_large": "https://download.pytorch.org/models/raft_large_C_T_V2-1bb1363a.pth"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: all current models use model_urls
transforms=RaftEval, | ||
meta={ | ||
"recipe": "https://github.com/princeton-vl/RAFT", | ||
"sintel_train_cleanpass_epe": 1.4411, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to rename one of them as the default epe
? This will allow you to add the metric in the schema of meta-data for optical flow models. It's also worth considering introducing a dictionary entry in the meta-data that holds other epe
values for for different datasets etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to rename one of them as the default epe?
Unfortunately no, because the rest of the weights will be trained on sintel, so reporting the epe on the trainset would not be relevant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to have a dict or something else to properly keep track of the other metrics though - ultimately I think it would make sense to also have 1px, 3px etc. I think we'll have a better idea of what it should look like once the rest of the weights are available
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, no strong opinions. You could dump all the metrics in an epe
dictionary. Then you would be able to include this on the schema. Up to you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @NicolasHug
…5060) Reviewed By: fmassa Differential Revision: D33185004 fbshipit-source-id: bdd968bd22775c2f63a8e67877b6482bfb58cc5a
Towards #4644
This PR:
--small
with--model
in the training ref as per Follow-up improvements to RAFT training reference #5056--weights
and--pretrained
in the training ref as per Follow-up improvements to RAFT training reference #5056The weights are trained on Chairs + Things and can be evaluated on the training set of Sintel or Kitti.
Some manual tests making sure all works fine:
Using
--weights Raft_Large_Weights.C_T_V2
Using
--pretrained
Using
--weights Raft_Large_Weights.C_T_V1
(Original weights)For my own sanity as for reference, here's the slurm script that I used (obviously these weights corresponds to the ones in things/raft-things.pth)
The code to map the original paper's weights to ours is
cc @datumbox