Trainers should ignore the ignore parameter

[calebrob6]

If I create a new class that extends SemanticSegmentationTask:

class CustomSemanticSegmentationTask(SemanticSegmentationTask):

    # any keywords we add here between *args and **kwargs will be found in self.hparams
    def __init__(self, *args, tmax=50, eta_min=1e-6, **kwargs) -> None:
        super().__init__(*args, **kwargs)  # pass args and kwargs to the parent class

Everything works great when I first instantiate the class

task = CustomSemanticSegmentationTask(model="unet", tmax=100, ...)

However, when I go to load a checkpoint from file:

task = CustomSemanticSegmentationTask.load_from_checkpoint("lightning_logs/version_3/checkpoints/epoch=0-step=117.ckpt")

I get an error:

TypeError: SemanticSegmentationTask.__init__() got an unexpected keyword argument 'ignore'

This happens because there is an ignore parameter stored in task.hparams, because SemanticSegmentationTask passes it to BaseTask, -- https://github.com/microsoft/torchgeo/blob/main/torchgeo/trainers/segmentation.py#L98.

I can add del kwargs["ignore"] before super().... in the constructor of CustomSemanticSegmentationTask but this feels like a bad hack, so I just make BaseTask ignore ignore as a hyperparameter.

[adamjstewart]

Alternatives:

1. Remove save_hyperparameters from BaseTask, add to all subclasses. But we use these hparams in configure_*, so we would have to move all of those too.
2. Add ignore as a parameter to all subclasses. Kind of feels silly to do this.
3. Add **kwargs as a parameter to all subclasses. Ignores any unknown keys, which is very bad in the case of typos.
4. Remove MoCo and SimCLR parameters that require ignoring.

EDIT: I think I have a better idea. Let me open a PR to demonstrate it.

[adamjstewart]

P.S. I don't actually think 4 is a horrible idea, as it isn't possible to restore these hparams from a checkpoint right now.

[calebrob6]

What's wrong with this fix?

[adamjstewart]

It feels like a workaround, that's all. It actually makes sense if you think about it, but it doesn't look as clean as what I'm now thinking of.

[calebrob6]

Ah yep, completely agree that it is a workaround

[calebrob6]

Closing in favor of #2317