Rename trainers

[adamjstewart]

The current naming of our trainers has led to much confusion. Here are some proposals for a new naming scheme, loosely ordered by my own personal preference.

Lightning-style

In PyTorch Lightning, there are LightningDataModules and LightningModules. In TorchGeo, there are GeoDataModules, and... you guessed it, GeoModules. The proposed naming scheme would look like:

from torchgeo.modules import Classification, Regression

Pros: consistency with PyTorch Lightning base class names
Cons: possible ambiguity between modules and nn.Module

Sklearn-style

PyTorch Lightning seems to be loosely designed around scikit-learn, which calls these objects estimators. Since there isn't a standard naming scheme introduced by Lightning, we could follow the sklearn philosophy instead. The proposed naming scheme would look like:

from torchgeo.estimators import Classifier, Regressor

Pros: no ambiguity with other names
Cons: wtf is an "estimator"?

Task-oriented

Our current trainers are organized around the tasks they try to solve. We could also name them as such:

from torchgeo.tasks import Classification, Regression

Pros: most similar to our current implementation
Cons: no other library does this

Trainer-oriented

This is our current implementation. Not changing anything is always an option:

from torchgeo.trainers import Classification, Regression

Pros: backwards compatible
Cons: ambiguity between trainers and pl.Trainer

Model-oriented

pl.Trainer takes two inputs, a model and a datamodule. We could call our trainers models:

from torchgeo.models import Classification, Regression

Pros: both models and trainers inherit from nn.Module
Cons: do I even need to explain how confusing this is?

Note that all of these can optionally have a Module, Task, or Trainer appended to the name. That's another thing to debate. Our data modules have DataModule appended to the name, but that's mostly to prevent name clashes between datasets and data modules. Our datasets/models/transforms don't do this, so I don't think our trainers should either.

This is obviously a big change to our API, and not something to take lightly. I don't want to make this change unless we can decide on a good evidence-based solution. If we just pick whichever one sounds right, we'll end up changing again in a year, which I don't want. I tried opening Lightning-AI/pytorch-lightning#14351 to discuss this but it seems like there is no community standard yet. So we also have the opportunity to define this standard ourselves.

[calebrob6]

The current naming of our trainers has led to much confusion

What confusion?

[adamjstewart]

The ambiguity: TorchGeo trainers != Lightning trainers (pl.Trainer)

It makes it difficult to talk about "trainers" since there are two kinds of trainers. Any conversation involving Lightning needs to first define what you mean by "trainer" and "model".

[calebrob6]

I don't think it is that confusing as pl.Trainer is only ever used in one context. I.e. you are never going to subclass it, just instantiate it and call .fit, etc.

[adamjstewart]

Yes, TorchGeo trainers and PL trainers are only ever used in one context, but they just so happen to be used in the same context.

[calebrob6]

More general response for each proposal above

• For "Lightning-style" naming I see the same ambiguity that currently exists between our trainers and pl.Trainer -- we would just be swapping terms to be ambiguous with nn.Module instead. This ambiguity would be much worse because each of our modules is a LightningModule which is in turn an nn.Module...
• For "Sklearn-style", nahhh. Re "wtf is an estimator" -- an estimator is actually a statistical term https://en.wikipedia.org/wiki/Estimator.
• For "Task-oriented", didn't we have this before? I like this one the best.
• For "Trainer-oriented", I don't mind our current implementation.
• For "Model-oriented", this is really confusing and shouldn't even be an option IMO.

[adamjstewart]

we would just be swapping terms to be ambiguous with nn.Module instead. This ambiguity would be much worse because each of our modules is a LightningModule which is in turn an nn.Module...

In my mind, the ambiguity is much less. Most users won't know or care that models, modules, and datamodules all subclass from nn.Module. But they will know that torchgeo.trainers and pl.Trainer are both the same name and completely unrelated because they have to import and use them in conjunction every time.

I like this one the best.

I figured most people would say that. I don't know how much of it is familiarity bias but I don't want to make a decision based on "this is how TorchGeo does it therefore this is how TorchGeo should do it" kind of logic. I wish there was a standard template that all projects should follow, but Lightning-AI/pytorch-lightning#14351 came up blank. I haven't yet found any other PyTorch domain libraries that add modules/datamodules.

[robmarkcole]

I also like task-oriented the best but think the current trainer-oriented is fine and not confusing.
Another option is lit_module