facebookresearch / facebookresearch/FlowDec

Bug: Incorrect RMSE normalization in per-band sigma_y calculation

Open
#11 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
215
Forks
21
PR merge metrics
No merged PRs in 30d

Description

In `estimate_flowdec_params.py`, the per-band RMSE calculation uses the wrong dimension for normalization, leading to underestimated `sigma_y` values.

**Location**

`estimate_flowdec_params.py`, line 165

**The Bug**

```python
# Current (INCORRECT):
torch.linalg.norm(diff.squeeze(), ord=2, dim=-1).cpu().numpy() / diff.shape[-2]**0.5

# Should be:
torch.linalg.norm(diff.squeeze(), ord=2, dim=-1).cpu().numpy() / diff.shape[-1]**0.5
```

**Explanation**

The L2 norm is computed along `dim=-1` (time dimension), but the code divides by `sqrt(diff.shape[-2])` (frequency bins) instead of `sqrt(diff.shape[-1])` (time frames).

For RMSE calculation:
- RMSE = L2_norm / sqrt(n), where `n` is the number of elements over which the norm is computed
- Since we compute the norm along the time axis (`dim=-1`), we should divide by `sqrt(time_frames)`
- `diff.shape[-1]` = time frames ✓
- `diff.shape[-2]` = frequency bins ✗

**Impact**

With typical parameters (nfft=1534, hop=384, 2-second samples at 48kHz):
- freq_bins ≈ 768
- time_frames ≈ 250
- **Error factor**: `sqrt(250/768) ≈ 0.57`

This means **`sigma_y` is underestimated by ~43%**.

**Note**: `beta` calculation is unaffected by this bug.

**Fix**

```diff
- torch.linalg.norm(diff.squeeze(), ord=2, dim=-1).cpu().numpy() / diff.shape[-2]**0.5
+ torch.linalg.norm(diff.squeeze(), ord=2, dim=-1).cpu().numpy() / diff.shape[-1]**0.5
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.