-
Notifications
You must be signed in to change notification settings - Fork 211
Conversation
if not model_kwargs: | ||
model_kwargs = {} | ||
|
||
model_kwargs["pretrained"] = pretrained |
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.
Need to add a check if the User is passing pretrained
in model_kwargs
, or else the default will be passed to the Model.
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.
Yes.
|
||
if isinstance(model, nn.Module): | ||
self.model = model | ||
elif isinstance(model, str): |
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.
Kindly add a check for _PYTORCH_VIDEO_AVAILABLE
here, or else self.models
will be an empty FlashRegistry.
Co-authored-by: Kaushik B <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #216 +/- ##
==========================================
- Coverage 87.38% 87.37% -0.02%
==========================================
Files 59 62 +3
Lines 3123 3294 +171
==========================================
+ Hits 2729 2878 +149
- Misses 394 416 +22
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
decoder=self.decoder, | ||
) | ||
if self.training: | ||
dataset.num_classes = len(np.unique([s[1]['label'] for s in ds._labeled_videos])) |
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.
This is a good little line of code, probably should move it to PTV and just expose a num_classes property (also means we wouldn't need to access the private _labeled_videos property. Will add this as a PTV issue and we can change it later.
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.
Great !
Hello @tchaton! Thanks for updating this PR. There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-04-30 14:35:36 UTC |
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.
This looks awesome 😍 the examples are super clean and easy to follow! Just some small comments 😃
@@ -0,0 +1,2 @@ | |||
from flash.video.classification.data import VideoClassificationData | |||
from flash.video.classification.model import VideoClassifier |
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.
add the preprocess
What does this PR do?
Fixes # (issue)
This PR integrates PyTorchVideo to Lightning Flash: https://pytorchvideo.org
Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃