google-research / google-research/flood-forecasting

[BUG] MaskedRMSELoss includes 0.5 factor inside sqrt, causing numerical divergence from standard RMSE metrics

Open Beginner friendly
#276 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.