-
Notifications
You must be signed in to change notification settings - Fork 487
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
[fix] Remove SmoothL1Loss from Tutorial #1298
Conversation
Model Benchmark
|
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #1298 +/- ##
=======================================
Coverage 89.85% 89.86%
=======================================
Files 38 38
Lines 5128 5131 +3
=======================================
+ Hits 4608 4611 +3
Misses 520 520
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@@ -793,6 +793,9 @@ def test_step(self, batch, batch_idx): | |||
loss, reg_loss = self.loss_func(inputs, predicted, targets) | |||
# Metrics | |||
if self.metrics_enabled: | |||
predicted_denorm = self.denormalize(predicted[:, :, 0]) | |||
target_denorm = self.denormalize(targets.squeeze(dim=2)) | |||
self.log_dict(self.metrics_val(predicted_denorm, target_denorm), **self.log_args) | |||
self.log("Loss_test", loss, **self.log_args) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to log (and eventually return) the error metrics specified by the user. those 3 lines are in validation_step and training_step, but (apparently) have been forgottten here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
🔬 Background
Closes #1234
SmoothL1Loss is not supported by Torchmetrics. With the lightning migration, we only use those, however.
Also,
m.test()
does not return error metrics specified by the user.🔮 Key changes
📋 Review Checklist