facebookresearch / facebookresearch/FlowDec
Bug: Incorrect RMSE normalization in per-band sigma_y calculation
- 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
Assessment
This issue has not been assessed yet.