Add visualisation callback for image classification #228

edgarriba · 2021-04-19T14:25:16Z

What does this PR do?

This PR implements the feature request from #74 and implements ImageClassificationDataVisualizer
to visualize the images and its associated labels right before feeding to the model.

Usage proposal:

   data_viz = ImageClassificationDataVisualizer.from_filepaths(
        train_filepaths=["path/img1.png", "path/img2.png"],
        train_labels=[0, 1],
        batch_size=2,
    )
    data_viz.show_train_batch()

Result obtained from the unittest:

TBD:

Exact information to be shown
Do we require at all matplotlib ?

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 🙃

edgarriba · 2021-04-19T14:31:11Z

tests/vision/classification/test_data.py

+    )
+    data_viz.show_train_batch()
+    data_viz.show_val_batch()
+    data_viz.show_test_batch()


we need to decide how we validate this functionality during tests since it involves matplotlib visualization

You could use: https://matplotlib.org/stable/devel/testing.html

flash/vision/classification/data.py

tchaton · 2021-04-19T15:08:21Z

tests/vision/classification/test_data.py

+    )
+    data_viz.show_train_batch()
+    data_viz.show_val_batch()
+    data_viz.show_test_batch()


You could use: https://matplotlib.org/stable/devel/testing.html

pep8speaks · 2021-04-20T12:56:31Z

Hello @edgarriba! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-04-22 11:54:46 UTC

codecov · 2021-04-20T17:02:18Z

Codecov Report

Merging #228 (f05c0d4) into master (7bfa80d) will increase coverage by 0.09%.
The diff coverage is 90.62%.

@@            Coverage Diff             @@
##           master     #228      +/-   ##
==========================================
+ Coverage   86.81%   86.90%   +0.09%     
==========================================
  Files          58       58              
  Lines        2981     3055      +74     
==========================================
+ Hits         2588     2655      +67     
- Misses        393      400       +7

Flag	Coverage Δ
unittests	`86.90% <90.62%> (+0.09%)`	⬆️

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

Impacted Files	Coverage Δ
flash/vision/classification/data.py	`87.98% <88.40%> (-0.23%)`	⬇️
flash/data/base_viz.py	`96.00% <88.88%> (-4.00%)`	⬇️
flash/data/data_module.py	`79.38% <100.00%> (+1.45%)`	⬆️
flash/utils/imports.py	`100.00% <100.00%> (ø)`

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 7bfa80d...f05c0d4. Read the comment docs.

tchaton

Overall LGTM ! Small changes needed.

tchaton · 2021-04-21T09:39:42Z

flash/data/data_module.py

@@ -92,6 +92,9 @@ def __init__(
        # this may also trigger data preloading
        self.set_running_stages()

+        # buffer to store the functions to visualise
+        self._fcn_white_list: Dict[str, Set[str]] = {}


This variable name isn't clean. Mind finding a name related to viz.

tchaton · 2021-04-21T09:45:18Z

flash/data/data_module.py

            data_fetcher._show(stage)
            if reset:
                self.viz.batches[stage] = {}

-    def show_train_batch(self, reset: bool = True) -> None:
+    def show_train_batch(self, name: str = 'load_sample', reset: bool = True) -> None:


List of hook names should also be supported.

I can provide the following api:

def show_train_batch(self, name: Union[str, List[str]] = 'load_sample', reset: bool = True) -> None: ...

tchaton · 2021-04-21T09:47:36Z

flash/data/data_module.py

        """This function is used to visualize a batch from the train dataloader."""
-        self._show_batch(_STAGES_PREFIX[RunningStage.TRAINING], reset=reset)
+        stage_name: str = _STAGES_PREFIX[RunningStage.TRAINING]


Move this logic to _show_batch to reduce duplicated code and raise a MisConfigurationError is the provided names aren't in _Preprocess_funcs.

tchaton · 2021-04-21T09:49:05Z

tests/data/test_callbacks.py

@@ -147,8 +144,9 @@ def configure_data_fetcher(*args, **kwargs) -> CustomBaseVisualization:

    for stage in _STAGES_PREFIX.values():

-        for _ in range(10):
-            getattr(dm, f"show_{stage}_batch")(reset=False)
+        for fcn_name in _PREPROCESS_FUNCS:


Keep the previous one, it was asserting we could iterate more than the dataset length and the iterator was being reset.

And let's add yours too.

tchaton · 2021-04-21T09:49:57Z

flash/vision/classification/data.py

@@ -521,3 +558,68 @@ def from_filepaths(
            seed=seed,
            **kwargs
        )
+
+
+class _MatplotlibVisualization(BaseVisualization):


Make this class public.

tchaton · 2021-04-21T09:50:21Z

flash/vision/classification/data.py

+    def configure_data_fetcher(*args, **kwargs) -> BaseDataFetcher:
+        return _MatplotlibVisualization(*args, **kwargs)
+
+    def show(self) -> None:


Let's find another way to do that. This is confusing.

we can have a better naming here. An alternative but a bit more hacky, is to pass blocking flag across all the different functions until the matplotlib plt.show calls.

tchaton · 2021-04-21T09:52:00Z

flash/vision/classification/data.py

+        rows: int = num_samples // cols
+
+        # create figure and set title
+        fig, axs = plt.subplots(rows, cols)


Raise an exception if matplotlib isn't available here: _MATPLOTLIB_AVAILABLE

done, check if that's appropriate

tchaton · 2021-04-21T09:57:45Z

flash/data/base_viz.py

+        # filter out the functions to visualise
+        func_name: str = self._fcn_white_list[running_stage]
+        func_names_list: Set[str] = list(set([func_name]) & set(_PREPROCESS_FUNCS))
+        if len(func_names_list) == 0:


Move this check the show_{}_batches in the DataModule.

ethanwharris

Looks good! small comments 😃

flash/data/base_viz.py

flash/data/data_module.py

flash/vision/classification/data.py

tchaton

Great ! Nicely done !

tchaton · 2021-04-22T11:33:40Z

flash/vision/classification/data.py

+        rows: int = num_samples // cols
+
+        if not _MATPLOTLIB_AVAILABLE:
+            raise MisconfigurationException("You need matplotlib to visualise. Please, pip install matplotlib")


Suggested change

raise MisconfigurationException("You need matplotlib to visualise. Please, pip install matplotlib")

raise MisconfigurationException("You need matplotlib to visualise. Please, use `pip install matplotlib`")

ethanwharris

Awesome! LGTM 😃

tchaton and others added 3 commits April 19, 2021 13:10

resolve bug

7850c17

update

dec2e16

draft version of the image classification callback

98b0564

edgarriba requested review from Borda, carmocca, justusschock, kaushikb11 and tchaton as code owners April 19, 2021 14:25

edgarriba commented Apr 19, 2021

View reviewed changes

Merge branch 'master' into feat/viz_callback

18f6339

tchaton reviewed Apr 19, 2021

View reviewed changes

Merge branch 'master' into feat/viz_callback

108ff3b

edgarriba marked this pull request as draft April 19, 2021 15:52

edgarriba added 7 commits April 19, 2021 21:23

move callback under base class

d2b423b

add crashing tests

c2587fd

add more tests

c2095be

more tests

b7d448d

more tests

3dd9b19

fix issues with files directory loading and add better error message

bd42635

fix visualisation test

32ac9b3

edgarriba force-pushed the feat/viz_callback branch from 176634c to dfdf333 Compare April 20, 2021 13:02

fix data outpye type

0ef5d2a

edgarriba force-pushed the feat/viz_callback branch from dfdf333 to 0ef5d2a Compare April 20, 2021 13:03

edgarriba added 3 commits April 20, 2021 18:39

fix tests with from_paths

d444841

add matplotlib check import

3a0b088

improve tests on from_folders

ce64c40

edgarriba added 2 commits April 20, 2021 19:09

add missing imports

be47408

fixed test_classification

b2a19bf

edgarriba added 7 commits April 20, 2021 21:44

implement all hooks

48ea3c2

fix more tests

76457f7

fix more tests

159c002

add matplotlib in reaquirements

b339bc9

remove useless test

0123284

Merge branch 'master' into feat/viz_callback

a48f149

implement function filtering to visualize

d125c5d

edgarriba marked this pull request as ready for review April 20, 2021 23:29

edgarriba requested a review from tchaton April 21, 2021 07:47

tchaton reviewed Apr 21, 2021

View reviewed changes

ethanwharris reviewed Apr 21, 2021

View reviewed changes

edgarriba mentioned this pull request Apr 21, 2021

Refactor preprocess_cls to preprocess, add Serializer, add DataPipelineState #229

Merged

8 tasks

edgarriba added 3 commits April 22, 2021 00:24

fix comments

21efbca

Merge branch 'master' into feat/viz_callback

9e8138f

add setter method to block windows to show with matplotlib

afeedac

edgarriba requested review from ethanwharris and tchaton April 22, 2021 10:55

tchaton approved these changes Apr 22, 2021

View reviewed changes

edgarriba added 2 commits April 22, 2021 13:48

sync with master and fix conflicts

0761bfb

remove unused variable

f05c0d4

ethanwharris approved these changes Apr 22, 2021

View reviewed changes

tchaton merged commit 1f9e151 into master Apr 22, 2021

tchaton deleted the feat/viz_callback branch April 22, 2021 12:08

edgarriba mentioned this pull request Apr 22, 2021

Add sample visualization option to the DataModule API #74

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add visualisation callback for image classification #228

Add visualisation callback for image classification #228

edgarriba commented Apr 19, 2021

edgarriba Apr 19, 2021

tchaton Apr 19, 2021

tchaton Apr 19, 2021

pep8speaks commented Apr 20, 2021 •

edited

Loading

codecov bot commented Apr 20, 2021 •

edited

Loading

tchaton left a comment

tchaton Apr 21, 2021

tchaton Apr 21, 2021

edgarriba Apr 21, 2021

tchaton Apr 21, 2021

edgarriba Apr 22, 2021

tchaton Apr 21, 2021

tchaton Apr 21, 2021

tchaton Apr 21, 2021

tchaton Apr 21, 2021

edgarriba Apr 22, 2021

tchaton Apr 21, 2021

edgarriba Apr 22, 2021

tchaton Apr 21, 2021

ethanwharris left a comment

tchaton left a comment

tchaton Apr 22, 2021

ethanwharris left a comment

	raise MisconfigurationException("You need matplotlib to visualise. Please, pip install matplotlib")
	raise MisconfigurationException("You need matplotlib to visualise. Please, use `pip install matplotlib`")

Add visualisation callback for image classification #228

Add visualisation callback for image classification #228

Conversation

edgarriba commented Apr 19, 2021

What does this PR do?

Before submitting

PR review

Did you have fun?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pep8speaks commented Apr 20, 2021 • edited Loading

Comment last updated at 2021-04-22 11:54:46 UTC

codecov bot commented Apr 20, 2021 • edited Loading

Codecov Report

tchaton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethanwharris left a comment

Choose a reason for hiding this comment

tchaton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethanwharris left a comment

Choose a reason for hiding this comment

pep8speaks commented Apr 20, 2021 •

edited

Loading

codecov bot commented Apr 20, 2021 •

edited

Loading