Skip to content

Fix feature_matching loss weight applied twice (400x instead of 20x) - #358

Open
taemincho wants to merge 1 commit into
acids-ircam:masterfrom
taemincho:fix/feature-matching-double-weight
Open

Fix feature_matching loss weight applied twice (400x instead of 20x)#358
taemincho wants to merge 1 commit into
acids-ircam:masterfrom
taemincho:fix/feature-matching-double-weight

Conversation

@taemincho

Copy link
Copy Markdown

Bug

In RAVE.training_step, entries added to loss_gen are already weighted at
construction time, e.g.:

loss_gen['feature_matching'] = self.weights['feature_matching'] * feature_matching_distance
loss_gen['adversarial'] = self.weights['adversarial'] * loss_adv

But the final accumulation loop re-applies the same weight via
self.weights.get(k, 1.):

loss_gen_value = 0.
for k, v in loss_gen.items():
    loss_gen_value += v * self.weights.get(k, 1.)

Any key in loss_gen whose name also exists in self.weights gets
double-weighted. With the default config, feature_matching's weight (20)
is effectively squared to 400x. adversarial (weight 1.0) is unaffected
numerically, and the spectral distance terms escape only because their
dict keys (multiband_spectral_distance, fullband_spectral_distance)
don't match self.weights's keys (multiband_audio_distance,
audio_distance).

This regression was introduced in 62a168a ("merge with last version +
normalization + bug fixes + input/output transforms"), which replaced the
previous, correct loss = sum(loss_gen.values(), 0) with the current loop,
without removing the pre-existing per-key weighting.

Fix

Since every entry in loss_gen is already weighted before insertion, the
final loop should just sum the values without re-weighting:

loss_gen_value = sum(loss_gen.values(), 0.)
loss_gen_value.backward()

This restores the original (pre-62a168a) behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant