Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Add semantic segmentation task #239

Merged
merged 58 commits into from
May 10, 2021
Merged

Add semantic segmentation task #239

merged 58 commits into from
May 10, 2021

Conversation

edgarriba
Copy link
Contributor

@edgarriba edgarriba commented Apr 22, 2021

What does this PR do?

This PR implements semantic segmentation task. It includes the following data structures:

  • SemantincSegmentationPreprocess
  • SemantincSegmentationData
  • SemantincSegmentation

some results:

image

TODO:

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests? [not needed for typos/docs]
  • Did you verify new and existing tests pass locally with your changes?
  • If you made a notable change (that affects users), did you update the CHANGELOG?

PR review

  • Is this pull request ready for review? (if not, please submit in draft mode)

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 🙃

@codecov
Copy link

codecov bot commented Apr 22, 2021

Codecov Report

Merging #239 (8745191) into master (cfb029e) will decrease coverage by 0.05%.
The diff coverage is 87.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #239      +/-   ##
==========================================
- Coverage   88.32%   88.26%   -0.06%     
==========================================
  Files          68       74       +6     
  Lines        3288     3563     +275     
==========================================
+ Hits         2904     3145     +241     
- Misses        384      418      +34     
Flag Coverage Δ
unittests 88.26% <87.54%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
flash/vision/segmentation/serialization.py 68.88% <68.88%> (ø)
flash/vision/segmentation/model.py 86.04% <86.04%> (ø)
flash/vision/segmentation/data.py 88.80% <88.80%> (ø)
flash/core/classification.py 92.75% <100.00%> (ø)
flash/data/batch.py 80.31% <100.00%> (+0.31%) ⬆️
flash/data/data_module.py 92.27% <100.00%> (+0.07%) ⬆️
flash/data/transforms.py 100.00% <100.00%> (ø)
flash/vision/__init__.py 100.00% <100.00%> (ø)
flash/vision/classification/data.py 93.87% <100.00%> (ø)
flash/vision/segmentation/__init__.py 100.00% <100.00%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cfb029e...8745191. Read the comment docs.

@pep8speaks
Copy link

pep8speaks commented Apr 23, 2021

Hello @edgarriba! Thanks for updating this PR.

Line 77:72: W504 line break after binary operator

Comment last updated at 2021-05-10 14:14:49 UTC

@edgarriba edgarriba requested a review from ethanwharris April 26, 2021 16:16
Comment on lines 81 to 118
@SemanticSegmentation.backbones(name="torchvision/fcn_resnet50")
def fn(pretrained: bool, num_classes: int):
import torchvision
model: nn.Module = torchvision.models.segmentation.fcn_resnet50(pretrained=pretrained, num_classes=num_classes)
return model
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe only register this if torchvision is installed?

@@ -18,38 +31,14 @@
from flash.data.process import Preprocess
from flash.utils.imports import _KORNIA_AVAILABLE, _MATPLOTLIB_AVAILABLE

from . import transforms as T
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we stay consistent with avoiding relative imports here?

flash/core/classification.py Outdated Show resolved Hide resolved
flash/core/classification.py Outdated Show resolved Hide resolved
flash/vision/segmentation/data.py Outdated Show resolved Hide resolved
flash/vision/segmentation/data.py Outdated Show resolved Hide resolved
flash/vision/segmentation/data.py Outdated Show resolved Hide resolved
flash/vision/segmentation/data.py Outdated Show resolved Hide resolved
flash/vision/segmentation/transforms.py Outdated Show resolved Hide resolved
@edgarriba edgarriba requested a review from Borda April 28, 2021 16:26
@ethanwharris ethanwharris changed the title Add semantic segmentation task [Blocked by #264] Add semantic segmentation task May 8, 2021
@ethanwharris ethanwharris changed the title [Blocked by #264] Add semantic segmentation task Add semantic segmentation task May 8, 2021
Copy link
Collaborator

@ethanwharris ethanwharris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 😃

Copy link
Contributor

@tchaton tchaton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good ! Small nits

docs/source/reference/semantic_segmentation.rst Outdated Show resolved Hide resolved
docs/source/reference/semantic_segmentation.rst Outdated Show resolved Hide resolved
flash/vision/segmentation/data.py Show resolved Hide resolved
flash/vision/segmentation/data.py Show resolved Hide resolved
flash/vision/segmentation/transforms.py Outdated Show resolved Hide resolved
flash/vision/segmentation/data.py Show resolved Hide resolved
test_transform=test_transform,
predict_transform=predict_transform,
data_sources={DefaultDataSources.PATHS: SemanticSegmentationPathsDataSource()},
default_data_source=DefaultDataSources.PATHS,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ethanwharris cannot this be overwritten by data_sources key ?

flash/vision/segmentation/transforms.py Outdated Show resolved Hide resolved
Copy link
Contributor

@tchaton tchaton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !

@ethanwharris ethanwharris enabled auto-merge (squash) May 10, 2021 13:58
@ethanwharris ethanwharris disabled auto-merge May 10, 2021 14:08
@ethanwharris ethanwharris enabled auto-merge (squash) May 10, 2021 14:14
@ethanwharris ethanwharris merged commit 719cf5c into master May 10, 2021
@ethanwharris ethanwharris deleted the feat/segmentation branch May 10, 2021 14:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants