-
Notifications
You must be signed in to change notification settings - Fork 863
[BUG] DeepAR: Inverted condition causes n_plotting_samples to remain None when n_validation_samples is None #2234
Description
Describe the bug
In the DeepAR model initialization, there is a logic error where the condition for setting n_plotting_samples is inverted, causing it to remain None when n_validation_samples is also None.
The buggy code in pytorch_forecasting/models/deepar/_deepar.py lines 143-147:
if n_plotting_samples is None:
if n_validation_samples is None:
n_plotting_samples = n_validation_samples # BUG: assigns None to None!
else:
n_plotting_samples = 100When both n_plotting_samples and n_validation_samples are None, the code assigns n_validation_samples (which is None) to n_plotting_samples, leaving it as None instead of assigning the default value of 100.
The condition if n_validation_samples is None: should be if n_validation_samples is not None:.
To Reproduce
from pytorch_forecasting.models import DeepAR
# Create DeepAR without specifying n_validation_samples or n_plotting_samples
# (Both default to None in function signature)
model = DeepAR(cell_type="LSTM")
# Check the resulting n_plotting_samples value
print(f"n_plotting_samples: {model.hparams.n_plotting_samples}")
# Output: n_plotting_samples: None
# Expected: n_plotting_samples: 100
# This None value is later used in create_log():
# n_samples = [self.hparams.n_validation_samples, self.hparams.n_plotting_samples][self.training]
# When training=True, n_samples will be None, which can cause downstream errorsExpected behavior
When n_validation_samples is None, the code should default n_plotting_samples to 100. The corrected code should be:
if n_plotting_samples is None:
if n_validation_samples is not None: # FIXED: inverted condition
n_plotting_samples = n_validation_samples
else:
n_plotting_samples = 100Additional context
The n_plotting_samples value is used in the create_log method (line 479) where it is passed as n_samples to prediction and quantiles functions. When None, this can cause issues in downstream calculations that expect an integer.
This bug affects all users who instantiate DeepAR without explicitly setting n_validation_samples or n_plotting_samples, which are both None by default in the function signature (line 65-66).
Versions
Details
System:
python: 3.12.0
executable: /Library/Frameworks/Python.framework/Versions/3.12/bin/python3
machine: macOS-26.3.1-arm64-arm-64bit
Python dependencies:
pip: 23.2.1
pytorch-forecasting: 1.6.1
torch: 2.9.1
lightning: 2.6.1
numpy: 2.3.5
scipy: 1.15.3
pandas: 2.3.3