Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change regression task to timm support #854

Merged
merged 13 commits into from
Dec 7, 2022

Conversation

nilsleh
Copy link
Collaborator

@nilsleh nilsleh commented Oct 16, 2022

This PR changes the RegressionTask to support Timm models instead of torchvision.models for greater model variety and consistency, because Timm is used in the other tasks as well.

  • new keyword argument num_outputs which allows the regression task to have multiple regression outputs, either if there are multiple regression targets or some form of probabilistic neural networks
  • other keyword arguments will be the same as ClassificationTask after Docs/trainers for Classification and Segmentation Task #852 is merged with the exception of the loss argument because regression only has mse loss at the moment

@github-actions github-actions bot added testing Continuous integration testing trainers PyTorch Lightning trainers labels Oct 16, 2022
@adamjstewart adamjstewart added this to the 0.4.0 milestone Oct 16, 2022
torchgeo/trainers/regression.py Outdated Show resolved Hide resolved
torchgeo/trainers/regression.py Outdated Show resolved Hide resolved
@calebrob6
Copy link
Member

Pulling out discussion from the comments, I think it makes most sense to call the deep learning model, "model", in the trainer code. In semantic segmentation tasks this will be a "unet", "deeplabv3+", or whatever else smp accepts, as that is the "model" that is being used, elsewhere this will be any timm architecture. In the segmentation trainer, it makes sense to call the encoder, "encoder".

I.e. what Adam said above but "encoder" instead of "backbone" (it makes sense to use "backbone" for BYOL and other SSL trainers).

@nilsleh
Copy link
Collaborator Author

nilsleh commented Nov 16, 2022

Pulling out discussion from the comments, I think it makes most sense to call the deep learning model, "model", in the trainer code. In semantic segmentation tasks this will be a "unet", "deeplabv3+", or whatever else smp accepts, as that is the "model" that is being used, elsewhere this will be any timm architecture. In the segmentation trainer, it makes sense to call the encoder, "encoder".

I.e. what Adam said above but "encoder" instead of "backbone" (it makes sense to use "backbone" for BYOL and other SSL trainers).

Let me know if this is what you had in mind and I understood you correctly. I will then open another PR to change the segmentation task names.

@calebrob6
Copy link
Member

Yep, exactly that!

Copy link
Collaborator

@adamjstewart adamjstewart left a comment

Choose a reason for hiding this comment

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

One minor change and I think this is ready to go.

torchgeo/trainers/regression.py Outdated Show resolved Hide resolved
adamjstewart
adamjstewart previously approved these changes Nov 27, 2022
Copy link
Collaborator

@adamjstewart adamjstewart left a comment

Choose a reason for hiding this comment

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

Would love to get one more review from someone before merging

@nilsleh
Copy link
Collaborator Author

nilsleh commented Nov 28, 2022

I.e. what Adam said above but "encoder" instead of "backbone" (it makes sense to use "backbone" for BYOL and other SSL trainers).

@adamjstewart looking back through the comments, I interpret the following:

  • Segmentation task: segmentation_model -> model and encoder_name -> encoder
  • Regression task: regression_model -> model
  • Classifcation task: classification_model -> model
  • BYOL task: encoder_name -> encoder (although Caleb seems to suggest encoder_name -> backbone?)
  • Object Detection: detection_model -> model and backbone -> ?

@adamjstewart
Copy link
Collaborator

Hmm, I would personally use "model" for the high-level model in all tasks and "encoder" iff that model has the ability to replace some kind of encoder/backbone with a pre-trained one. I guess I don't see a big distinction between SL encoders and SSL backbones, but I'll let @calebrob6 clarify.

@calebrob6
Copy link
Member

Encoder or backbone is fine for BYOL

@nilsleh
Copy link
Collaborator Author

nilsleh commented Nov 28, 2022

Okay so,

  • Segmentation task: segmentation_model -> model and encoder_name -> encoder
  • Regression task: regression_model -> model
  • Classifcation task: classification_model -> model
  • BYOL task: encoder_name -> encoder
  • Object Detection: detection_model -> model

is the final verdict?

@calebrob6
Copy link
Member

LGTM!

@adamjstewart adamjstewart enabled auto-merge (squash) December 4, 2022 19:57
auto-merge was automatically disabled December 6, 2022 21:55

Pull request was closed

@adamjstewart adamjstewart reopened this Dec 6, 2022
@adamjstewart adamjstewart merged commit d6d568a into microsoft:main Dec 7, 2022
yichiac pushed a commit to yichiac/torchgeo that referenced this pull request Apr 29, 2023
* change regression task to timm support

* add docstring about available models

* typo again

* failing test

* change name

* change name

* expose all available models

* docstring list_models

* Update torchgeo/trainers/regression.py

Co-authored-by: Caleb Robinson <calebrob6@gmail.com>
Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Continuous integration testing trainers PyTorch Lightning trainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants