Claude found some issues with CoherenceRegularization:
- Cross-PSD between forecast and observation is never computed
Docstring promises:
CrossPSD_l = sum_{c,m} w_m * Re( f^(e){c,l,m} * conj(y{c,l,m}) )
Coh_l = CrossPSD_l^2 / (PSD^f_l * PSD^y_l + eps)
Loss = (1 / |band|) * sum_{l in band} (1 - mean_{e} Coh_l^(e))
i.e. measure how well each ensemble member's spectral phases align with the observation's, in a wavenumber band.
What forward computes instead (line 320):
coh_fcasts = (m_weights * (forecasts.unsqueeze(0).conj() * forecasts.unsqueeze(1)).real).sum(dim=-1)
That's forecast × forecast — the inter-member coherence. The forecast-versus-observation cross-PSD that the docstring describes is never calculated. The actual loss ends up being a
psd_obs-weighted inter-member decoherence penalty, more like the spread term of SpectralCoherenceLoss than the forecast-to-truth coherence the name implies.
- The wavenumber-band mask is dead
The whole point of the class — "configurable wavenumber band [l_min, l_max], targeting the mesoscale range" — does nothing. lmin/lmax get used to build self.l_band (line 270,
registered as a buffer at line 281), but forward never multiplies by it. The loss sums over all l, not the configured band.
- ensemble_coherence_weight is dead
Stored as self.ensemble_coherence_weight on line 250, never read anywhere else in the file. The docstring's "Optionally adds an inter-member coherence penalty" doesn't have a
corresponding code path.
Claude found some issues with CoherenceRegularization:
Docstring promises:
CrossPSD_l = sum_{c,m} w_m * Re( f^(e){c,l,m} * conj(y{c,l,m}) )
Coh_l = CrossPSD_l^2 / (PSD^f_l * PSD^y_l + eps)
Loss = (1 / |band|) * sum_{l in band} (1 - mean_{e} Coh_l^(e))
i.e. measure how well each ensemble member's spectral phases align with the observation's, in a wavenumber band.
What forward computes instead (line 320):
coh_fcasts = (m_weights * (forecasts.unsqueeze(0).conj() * forecasts.unsqueeze(1)).real).sum(dim=-1)
That's forecast × forecast — the inter-member coherence. The forecast-versus-observation cross-PSD that the docstring describes is never calculated. The actual loss ends up being a
psd_obs-weighted inter-member decoherence penalty, more like the spread term of SpectralCoherenceLoss than the forecast-to-truth coherence the name implies.
The whole point of the class — "configurable wavenumber band [l_min, l_max], targeting the mesoscale range" — does nothing. lmin/lmax get used to build self.l_band (line 270,
registered as a buffer at line 281), but forward never multiplies by it. The loss sums over all l, not the configured band.
Stored as self.ensemble_coherence_weight on line 250, never read anywhere else in the file. The docstring's "Optionally adds an inter-member coherence penalty" doesn't have a
corresponding code path.