Segformer swi #1292

Yael-Baron · 2023-07-18T12:18:49Z

No description provided.

Louis-Dupont

Is the PR for review ?
I had a look anyway and left some comments, but just minor notes on naming/doc

src/super_gradients/training/models/segmentation_models/segformer.py

src/super_gradients/training/utils/callbacks/callbacks.py

src/super_gradients/training/models/segmentation_models/segformer.py

shaydeci

Comments inline, about some we have already had a discussion (so just a reminder).
Still missing:

Unit tests - see inline.
Integration tests, pretrained model urls.
New phase for the final valdiation on the average model.

src/super_gradients/training/models/segmentation_models/segformer.py

shaydeci · 2023-07-20T08:30:42Z

src/super_gradients/common/object_names.py

@@ -148,6 +148,7 @@ class Callbacks:
    DEKR_VISUALIZATION = "DEKRVisualizationCallback"
    ROBOFLOW_RESULT_CALLBACK = "RoboflowResultCallback"
    TIMER = "TimerCallback"
+    SLIDING_WINDOW_INFERENCE = "ChangeToSWI"


Is this the name of the callback?

Fixed on c79d0f2

shaydeci · 2023-07-20T08:31:49Z

src/super_gradients/training/utils/callbacks/callbacks.py

@@ -856,6 +856,22 @@ def _infer_global_step(self, context: PhaseContext, is_train_loader: bool):
            return total_steps_in_done + train_loader_length + context.batch_idx


+@register_callback(Callbacks.SLIDING_WINDOW_INFERENCE)


Why choose an inconsistent name? This just adds more confusion....

Fixed on c79d0f2

src/super_gradients/training/utils/callbacks/callbacks.py

src/super_gradients/training/models/segmentation_models/segformer.py

BloodAxe · 2023-07-31T07:39:00Z

src/super_gradients/training/utils/callbacks/callbacks.py

+    def on_validation_loader_start(self, context: PhaseContext) -> None:
+        if context.training_params.max_epochs - 1 == context.epoch:
+            unwrap_model(context.net).enable_swi()
+            context.valid_loader.dataset.transforms.transforms = []


I feel that emptying transforms list in this way is VERY risky move. That may be solving immediate goal of adding SWI but if someone who is less familiar with implementation details runs into troubles because of this change - the odds they would curse us are pretty high.
Maybe even now we may have some incompatible scenarios where this would break (Like quantization). I really feel uneasy about this line. Is there really not other way of solving this?

I agree. It was a quick fix due to time frame concerns.
@shaydeci how should I proceed?

Missed this one, completely agree with @BloodAxe here.
How about mentioning explicitly the new transforms we switch to when switching to SWI? Is it always going to be the case that we just empty the transforms ?
It does not make sense to me that transforms that might have nothing to do with resolution should be dropped...

Any way, we should set the transforms taken out after the validation is completed.
I think an appropriate warning/message should be printed as well.

If we drop transforms to keep spatial size, how do we handle normalization transforms then?

Ithink we need to call iter otherwise this won't be updated. Have you checked that images actually pass through this empty pipeline ?

Since wach worker has its own instance of the dataset, it is required to call iter on it, this is why in our training stage switch callback for YoloX when we want to turn of the transforms:

@register_callback(Callbacks.YOLOX_TRAINING_STAGE_SWITCH) class YoloXTrainingStageSwitchCallback(TrainingStageSwitchCallbackBase): """ YoloXTrainingStageSwitchCallback Training stage switch for YoloX training. Disables mosaic, and manipulates YoloX loss to use L1. """ def __init__(self, next_stage_start_epoch: int = 285): super(YoloXTrainingStageSwitchCallback, self).__init__(next_stage_start_epoch=next_stage_start_epoch) def apply_stage_change(self, context: PhaseContext): for transform in context.train_loader.dataset.transforms: if hasattr(transform, "close"): transform.close() iter(context.train_loader) context.criterion.use_l1 = True

Fixed on 0ec8a9c

src/super_gradients/training/models/segmentation_models/segformer.py

…o have a consistent name for clarity. Added a new phase for the final validation on the average model. Added docstrings

…egformer recipe.

Yael-Baron · 2023-07-31T11:01:36Z

Pushed a new commit (no. 1b8a739) with new recipes for each segformer's variant based on a default recipe.

src/super_gradients/training/models/segmentation_models/segformer.py

… support sliding window inference.

…rted to tuples.

…od set in segmentation_utils.py that could also be used in other models.

…ngWindowValidationCallback.

…was added to the test suit.

…ow inference.

Yael-Baron · 2023-08-06T13:57:16Z

Pushed integration test for segformer's models on commit db42e6c

shaydeci · 2023-08-07T10:48:27Z

tests/unit_tests/pretrained_models_unit_test.py

@@ -29,6 +29,36 @@ def test_pretrained_repvgg_a0_imagenet(self):
        model = models.get(Models.REPVGG_A0, pretrained_weights="imagenet", arch_params={"build_residual_branches": True})
        trainer.test(model=model, test_loader=classification_test_dataloader(), test_metrics_list=[Accuracy()], metrics_progress_verbose=True)

+    def test_pretrained_segformer_b0_cityscapes(self):


These tests are a bit meaningless. You can take them out.

shaydeci

LGTM on my end.

BloodAxe · 2023-08-07T11:14:37Z

src/super_gradients/training/utils/segmentation_utils.py

@@ -43,6 +43,10 @@ def to_one_hot(target: torch.Tensor, num_classes: int, ignore_index: int = None)
    :param num_classes: num of classes in datasets excluding ignore label, this is the output channels of the one hot
        result.
    :return: one hot tensor with shape [N, num_classes, H, W]
+
+    Parameters


Why

Parameters -----------

and not :param ignore index?

BloodAxe · 2023-08-07T11:15:48Z

src/super_gradients/training/utils/callbacks/callbacks.py

+    def on_test_loader_end(self, context: PhaseContext) -> None:
+        unwrap_model(context.net).disable_sliding_window_validation()
+        context.test_loader.dataset.transforms.transforms = self.test_loader_transforms
+        iter(context.test_loader)


I'm not sure I'm following why we need iter(context.test_loader) here.

Yael-Baron · 2023-08-10T07:52:59Z

A signed PR was opened, please follow Segformer swi signed #1361 PR.

* Squashed version of branch Segformer_SWI (see Segformer SWI #1292 PR ) * Docstrings fix * Docstrings fix --------- Co-authored-by: Louis-Dupont <35190946+Louis-Dupont@users.noreply.github.com>

Yael-Baron added 2 commits July 18, 2023 15:12

Sliding window callback for SegFormer model.

64cd1b6

Removed callback __init__

6a879c0

Yael-Baron requested review from shaydeci, ofrimasad, BloodAxe and Louis-Dupont as code owners July 18, 2023 12:18

Louis-Dupont reviewed Jul 19, 2023

View reviewed changes

shaydeci suggested changes Jul 20, 2023

View reviewed changes

Merge branch 'master' into segformer_SWI

4b40860

BloodAxe reviewed Jul 31, 2023

View reviewed changes

src/super_gradients/training/models/segmentation_models/segformer.py Outdated Show resolved Hide resolved

BloodAxe reviewed Jul 31, 2023

View reviewed changes

src/super_gradients/training/models/segmentation_models/segformer.py Outdated Show resolved Hide resolved

BloodAxe reviewed Jul 31, 2023

View reviewed changes

src/super_gradients/training/models/segmentation_models/segformer.py Show resolved Hide resolved

Yael-Baron added 3 commits July 31, 2023 11:34

Changed the sliding window callback's name and all its dependencies t…

c79d0f2

…o have a consistent name for clarity. Added a new phase for the final validation on the average model. Added docstrings

Merge remote-tracking branch 'origin/segformer_SWI' into segformer_SWI

80c3f1b

A recipe for each variant was created based on a default cityscapes_s…

1b8a739

…egformer recipe.

Yael-Baron added 2 commits August 1, 2023 10:55

Models were uploaded to the model zoo.

6c9b1a8

Added unit test for segformer

8ff0b9b

BloodAxe reviewed Aug 1, 2023

View reviewed changes

src/super_gradients/training/models/segmentation_models/segformer.py Outdated Show resolved Hide resolved

BloodAxe reviewed Aug 1, 2023

View reviewed changes

src/super_gradients/training/models/segmentation_models/segformer.py Outdated Show resolved Hide resolved

Yael-Baron and others added 9 commits August 1, 2023 15:20

An exception is raised at the beginning of training if model does not…

f400bf3

… support sliding window inference.

A correction was made in the docstring of SegFormer's __init__

2821ee9

Objects sliding_window_crop_size and sliding_window_stride were conve…

4e91c8d

…rted to tuples.

Refactoring the forward_with_sliding_window logic to an external meth…

1475d13

…od set in segmentation_utils.py that could also be used in other models.

Adding a parameter called transforms_for_sliding_window for the Slidi…

0ec8a9c

…ngWindowValidationCallback.

Small improvements.

8a178fa

Merge branch 'master' into segformer_SWI

58e8696

Fix for commit f400bf3

ec450ec

Merge remote-tracking branch 'origin/segformer_SWI' into segformer_SWI

9c7ff0f

Yael-Baron added 5 commits August 6, 2023 09:00

SlidingWindowTest unit test from forward_with_sliding_window_test.py …

61ea395

…was added to the test suit.

Fixed check if model supports sliding window inference.

e2595b5

Added a unit test for sliding window inference.

e9a6e9f

Added integration tests for the segformer's models using sliding wind…

db42e6c

…ow inference.

Small fix

4c518de

shaydeci suggested changes Aug 7, 2023

View reviewed changes

shaydeci approved these changes Aug 7, 2023

View reviewed changes

BloodAxe reviewed Aug 7, 2023

View reviewed changes

Merge branch 'master' into segformer_SWI

fb5b620

Yael-Baron mentioned this pull request Aug 10, 2023

Segformer swi signed #1361

Merged

Yael-Baron closed this Aug 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Segformer swi #1292

Segformer swi #1292

Yael-Baron commented Jul 18, 2023

Louis-Dupont left a comment

shaydeci left a comment

shaydeci Jul 20, 2023

Yael-Baron Jul 31, 2023

shaydeci Jul 20, 2023

Yael-Baron Jul 31, 2023

BloodAxe Jul 31, 2023

Yael-Baron Jul 31, 2023

shaydeci Jul 31, 2023

BloodAxe Aug 1, 2023

shaydeci Aug 1, 2023 •

edited

Loading

Yael-Baron Aug 2, 2023

Yael-Baron commented Jul 31, 2023

Yael-Baron commented Aug 6, 2023

shaydeci Aug 7, 2023

shaydeci left a comment

BloodAxe Aug 7, 2023

BloodAxe Aug 7, 2023

Yael-Baron commented Aug 10, 2023

		@@ -856,6 +856,22 @@ def _infer_global_step(self, context: PhaseContext, is_train_loader: bool):
		return total_steps_in_done + train_loader_length + context.batch_idx


		@register_callback(Callbacks.SLIDING_WINDOW_INFERENCE)

Segformer swi #1292

Segformer swi #1292

Conversation

Yael-Baron commented Jul 18, 2023

Louis-Dupont left a comment

Choose a reason for hiding this comment

shaydeci 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

shaydeci Aug 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yael-Baron commented Jul 31, 2023

Yael-Baron commented Aug 6, 2023

Choose a reason for hiding this comment

shaydeci left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yael-Baron commented Aug 10, 2023

shaydeci Aug 1, 2023 •

edited

Loading