Fix boolean operator precedence bugs in GeneratorLoss#4404
Closed
Mr-Neutr0n wants to merge 1 commit intocoqui-ai:devfrom
Closed
Fix boolean operator precedence bugs in GeneratorLoss#4404Mr-Neutr0n wants to merge 1 commit intocoqui-ai:devfrom
Mr-Neutr0n wants to merge 1 commit intocoqui-ai:devfrom
Conversation
Fix two boolean logic errors in GeneratorLoss.forward() caused by Python operator precedence: 1. `not scores_fake is not None` evaluated as `(not scores_fake) is not None`, which is always True since bool values are never None. This meant the hinge loss block ran unconditionally (even when scores_fake was None), causing AttributeError at runtime. 2. `not feats_fake is None` evaluated as `(not feats_fake) is None`, which is always False since bool values are never None. This meant the feature matching loss was silently never applied. Both conditions now use the idiomatic `x is not None` form, consistent with the existing MSE loss check on the preceding line.
sapreparth
approved these changes
Mar 14, 2026
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. You might also look our discussion channels. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
not scores_fake is not Nonewas parsed as(not scores_fake) is not Nonedue to Python operator precedence, which is alwaysTrue— the hinge loss block ran unconditionally even whenscores_fakewasNone, causingAttributeErrorat runtime.not feats_fake is Nonewas parsed as(not feats_fake) is None, which is alwaysFalse— the feature matching loss was silently never applied.x is not Noneform, consistent with the existing MSE loss check on the preceding line.Details
In
TTS/vocoder/layers/losses.py, theGeneratorLoss.forward()method had two boolean expressions affected by Python operator precedence:not scores_fake is not None(not scores_fake) is not NoneTrue(bool is never None)not feats_fake is None(not feats_fake) is NoneFalse(bool is never None)The
notunary operator binds more tightly thanis/is not, so these expressions do not test what was intended.After fix: both lines use
scores_fake is not Noneandfeats_fake is not None, matching the pattern already used for the MSE loss guard on line 289.Test plan
scores_fakeis providedfeats_fakeis providedpython -m pytest tests/vocoder_tests/