-
Notifications
You must be signed in to change notification settings - Fork 647
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
Multiple Target Prediction Plotting Bug #1317
base: main
Are you sure you want to change the base?
Conversation
Is this repo not maintained? |
@@ -1213,7 +1217,7 @@ def configure_optimizers(self): | |||
min_lr=self.hparams.reduce_on_plateau_min_lr, | |||
), | |||
"monitor": "val_loss", # Default: val_loss | |||
"interval": "epoch", | |||
"interval": "step", |
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.
don't think this makes sense. reduce each step is very aggressive
capsize=1.0, | ||
) | ||
except ValueError: | ||
print(f"Warning: could not plot error bars. Quantiles: {quantiles}, y: {y}, yerr: {quantiles - y[-n_pred]}") |
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 would have to be a logger.warning()
instead of print()
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.
why does the error actually happen. Seems like this solves a different problem
@@ -1012,7 +1013,7 @@ def plot_prediction( | |||
# move to cpu | |||
y = y.detach().cpu() | |||
# create figure | |||
if ax is None: | |||
if (ax is None) or (not ax_provided): |
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 confusing. I would say: ax_not_provided = ax is None
before the loop and then if ax_not_provided
after the loop
Codecov ReportPatch coverage:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1317 +/- ##
==========================================
- Coverage 90.13% 90.05% -0.08%
==========================================
Files 30 30
Lines 4712 4716 +4
==========================================
Hits 4247 4247
- Misses 465 469 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Description
When calling the plot_prediction() function in PytorchForecasting with multiple targets, the function reuses the same axes for each target. This behavior results in overlapped plots for different targets, rather than separate plots for each target.
This pull request fixes this issue.
Issue
#1314
The faulty code part
Expected behavior:
Each target should be plotted on a separate figure.
Actual behavior:
All targets are plotted on the same figure, resulting in overlapped plots.
Solution
In the above snippet, the variable ax should be updated within the loop over targets but instead after the first target, the same ax is reused (as ax is no longer None). The proposed issue fix is:
Bonus
Corrected mistakes in documentation. The encoder's
log1p
transformation is incorrectly calledlogp1
in the documentation.#1247