google-research / google-research/flood-forecasting

[Bug] sample_cmal_deterministic ignores negative_sample_handling config and produces negative streamflow values

Open
#277 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
347
Forks
92
Avg merge
8d 7h
Merged PRs (30d)
4

Description

### Description
When a model is configured with `negative_sample_handling: 'clip'`, predictions are expected to enforce physical non-negativity ($Q \ge 0.0$).

While `sample_cmal()` correctly ingests the dataset `Scaler`, calculates the normalized zero threshold, and applies `_handle_negative_values()`, `sample_cmal_deterministic()` omits the `scaler` parameter and bypasses negative sample handling entirely. Consequently, deterministic CMAL forecasts
produce negative streamflow predictions when unscaled.

In `test/test_uncertainty.py:137-144`, this discrepancy was bypassed by explicitly exempting `cmal_deterministic` from the non-negativity check:

```python
if (
negative_sample_handling == 'clip'
and config.head.lower() != 'cmal_deterministic'
):
assert np.allclose(negative_vals, 0.0, atol=1e-6)
```
### Steps to Reproduce

1. Configure a run with head: 'cmal_deterministic' and negative_sample_handling: 'clip'.
2. Generate predictions during low-flow periods where lower quantiles (q = 0.1,q = 0.2) fall below normalized zero (-center/scale).
3. Unscale predictions to physical space (m³/s).
4. Observed: Lower quantiles evaluate to negative streamflow discharge.
5. Expected: Values below normalized zero are clamped, producing non-negative streamflow (≥0.0).

### Affected Files

• googlehydrology/utils/samplingutils.py (lines 378-415)
• test/test_uncertainty.py (lines 137-144)

### Proposed Fix

1. Update sample_cmal_deterministic in googlehydrology/utils/samplingutils.py to accept scaler: Scaler | None = None and execute _handle_negative_values():
```python
def sample_cmal_deterministic(
model: 'BaseModel',
data: dict[str, torch.Tensor],
*,
scaler: Scaler | None = None,
outputs: dict[str, torch.Tensor] | None = None,
) -> dict[str, torch.Tensor]:
# ... generate sample points ...
if scaler is not None and setup.cfg.negative_sample_handling:
normalized_zero = _calc_normalized_zero_thresholds(
scaler=scaler,
targets=setup.cfg.target_variables,
device=setup.device,
dtype=samples[f'y_hat{setup.freq_suffixes[0]}'].dtype,
)
for freq_suffix in setup.freq_suffixes:
key = f'y_hat{freq_suffix}'
samples[key] = _handle_negative_values(
cfg=setup.cfg,
values=samples[key],
sample_values=lambda ids: samples[key][ids],
normalized_zero=normalized_zero,
)
return samples
```
2. Pass scaler through sample_pointpredictions() at samplingutils.py:65.
3. Remove the and config.head.lower() != 'cmal_deterministic' exception from test/test_uncertainty.py:139.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in googlehydrology/utils/samplingutils.py, reading sample_pointpredictions() and sample_cmal_deterministic(), then inspect test/test_uncertainty.py:137-144. Run the uncertainty tests with cmal_deterministic and negative_sample_handling set to clip. Done means the deterministic path receives the scaler, enforces non-negative unscaled streamflow, and passes after the exemption is removed.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, pytorch
Domain
machine-learning, testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.