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

🔨 Rework for Probabilistic Regression & Pixel Regression #125

Merged
merged 21 commits into from
Dec 18, 2024

Conversation

alafage
Copy link
Contributor

@alafage alafage commented Nov 28, 2024

  • Rework Distribution Layers to be classifiers returning distribution parameters in a dictionary.
  • Update RegressionRoutine
  • Update PixelRegressionRoutine
  • Update Tutorials
  • Update API Reference
  • Fix DistributionNLL (loss / metric)
  • Update BTS baseline
  • Add tutorial for simple probabilistic regression
  • Update existing tests
  • Write tests

Copy link

codecov bot commented Nov 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.22%. Comparing base (86eec13) to head (12b2f1c).
Report is 26 commits behind head on dev.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #125   +/-   ##
=======================================
  Coverage   99.21%   99.22%           
=======================================
  Files         142      142           
  Lines        6792     6835   +43     
  Branches      849      857    +8     
=======================================
+ Hits         6739     6782   +43     
  Misses         24       24           
  Partials       29       29           
Flag Coverage Δ
cpu 99.22% <100.00%> (+<0.01%) ⬆️
pytest 99.22% <100.00%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alafage alafage added the enhancement New feature or request label Nov 28, 2024
@alafage alafage self-assigned this Nov 28, 2024
@alafage alafage marked this pull request as ready for review December 17, 2024 16:20
@alafage alafage requested a review from o-laurent December 17, 2024 16:21
Copy link
Contributor

@o-laurent o-laurent left a comment

Choose a reason for hiding this comment

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

Some comments on the documentation mainly!
Thanks for improving this part that wasn't that great (Sorry 🤐 )

torch_uncertainty/layers/distributions.py Show resolved Hide resolved
torch_uncertainty/layers/distributions.py Show resolved Hide resolved
torch_uncertainty/layers/distributions.py Show resolved Hide resolved
)
self.min_scale = min_scale

def forward(self, x: Tensor) -> dict[str, Tensor]:
Copy link
Contributor

Choose a reason for hiding this comment

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

📖

torch_uncertainty/layers/distributions.py Show resolved Hide resolved

Args:
x (Tensor): The input of the model.

Returns:
Copy link
Contributor

Choose a reason for hiding this comment

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

Detail the return type?

loss (nn.Module): Loss function to optimize the :attr:`model`.
dist_family (str, optional): The distribution family to use for
probabilistic pixel regression. Defaults to ``None``.
Copy link
Contributor

Choose a reason for hiding this comment

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

If None, pointwise regression

@@ -76,7 +79,9 @@ def __init__(
)

self.model = model
self.probabilistic = probabilistic
self.dist_family = dist_family
self.dist_estimate = dist_estimate
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is self.dist_estimate here and not in pixel-wise regression?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a mistake, thanks!

torch_uncertainty/layers/distributions.py Show resolved Hide resolved
@alafage
Copy link
Contributor Author

alafage commented Dec 18, 2024

Thank you for the great review! This last commit should address all your worries. Feel free to merge it into dev anytime!

Copy link
Contributor

@o-laurent o-laurent left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! Just saw a small typo but we're good to go!

Depending of the :attr:`dist_layer` of the backbone, the output can
be a distribution or a single tensor.
Depending of the :attr:`dist_family` of the backbone, the output can
be a dictionnary of distribution parameters or a single tensor.
Copy link
Contributor

Choose a reason for hiding this comment

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

dictionary

@o-laurent o-laurent merged commit 7747a7a into dev Dec 18, 2024
1 check passed
@alafage alafage deleted the density_layers branch December 18, 2024 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants