google-research / google-research/flood-forecasting
[BUG] MaskedRMSELoss includes 0.5 factor inside sqrt, causing numerical divergence from standard RMSE metrics
- Dominant language
- Python
- Stars
- 343
- Forks
- 87
- Avg merge
- 14d 23m
- Merged PRs (30d)
- 1
Description
**Location**
`googlehydrology/training/loss.py:306-311`, `test/test_losses.py:127`
**Root Cause Mechanism**
`MaskedRMSELoss._get_loss` applies a $0.5$ factor inside the radical:
$$\text{loss} = \sqrt{0.5 \times \text{mean}\left((\hat{y} - y)^2\right)} = \frac{1}{\sqrt{2}}\,\text{RMSE} \approx 0.7071 \times \text{RMSE}$$
While the $\frac{1}{2}$ multiplier is standard in Mean Squared Error objectives ($\frac{1}{2}\text{MSE}$) to cancel the exponent during differentiation, placing it inside the square root scales the loss value down by $\approx 29.3\%$. Consequently, it mismatches `googlehydrology.evaluation.metrics.rmse`, which computes standard $\sqrt{\text{MSE}}$ without the scaling factor.
**Impact**
Training with `loss: 'rmse'` optimizes an artificially attenuated loss landscape, producing loss curves and training checkpoints whose numerical magnitudes diverge from all standard validation and benchmark evaluation metrics.
**Suggested Fix**
Remove the `0.5 *` coefficient in `MaskedRMSELoss._get_loss` and align the expected baseline in `test/test_losses.py`:
```python
loss = torch.sqrt(
torch.mean((prediction['y_hat'][mask] - ground_truth['y'][mask]) ** 2)
)
```
**Verification Checklist**
* [ ] Update test assertions in `test/test_losses.py:127` to reflect standard unscaled RMSE values.
* [ ] Confirm that `MaskedRMSELoss` output exactly matches `googlehydrology.evaluation.metrics.rmse` on identical masked tensors.
* [ ] Verify learning rate dynamics or optimizer schedulers configured for RMSE loss runs to account for the $\approx 1.414\times$ change in effective loss and gradient magnitude.
Contributor guide
Research direction
Start at googlehydrology/training/loss.py:306-311 and compare MaskedRMSELoss with googlehydrology.evaluation.metrics.rmse. Update the expected assertion at test/test_losses.py:127, then run the loss tests and confirm the masked loss matches the standard RMSE on identical tensors. Done means the coefficient is removed and the tests pass with unscaled RMSE values.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, pytorch
- Domain
- machine-learning, testing-qa
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 84/100