Skip to content

FS2 Loss included padded frames - #887

Merged
roedoejet merged 1 commit into
mainfrom
dev.ap/fs2-loss
Oct 15, 2026
Merged

roedoejet merged 1 commit into
mainfrom
dev.ap/fs2-loss

Conversation

@roedoejet

@roedoejet roedoejet commented Sep 16, 2026 •

Copy link
Copy Markdown
Member

PR Goal?

Previously, the loss was calculated by multiplying against the mask, but this would still include the padded frames in MSE loss, unnecessarily. Here we switch to masked select instead to only calculate loss on non-padded inputs/outputs.

Fixes?

Issue raised by personal communication with @korin-richmond

Feedback sought?

sanity

Priority?

medium

Tests added?

yes

How to test?

I've included tests. you should be able to train a new model or finetune existing models without issues

Confidence?

high

Version change?

patch

Related PRs?

EveryVoiceTTS/FastSpeech2_lightning#166

@semanticdiff-com

semanticdiff-com Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Review changes with  SemanticDiff

Changed Files
File Status
  everyvoice/model/feature_prediction/FastSpeech2_lightning  0% smaller

@roedoejet
roedoejet requested a review from joanise September 16, 2026 17:32
@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor
CLI load time: 0:00.24
Pull Request HEAD: 3f62c989ac036ba0466a5618ef8703ab57e8e2b0

Imports that take more than 0.1 s:
import time: self [us] | cumulative | imported package
import time:       698 |     118834 |   typer
import time:      7046 |     241091 | everyvoice.cli
import time:      2121 |     116999 |       markdown_it.main
import time:       408 |     117406 |     markdown_it
import time:      5766 |     168559 |   rich.markdown
import time:      4828 |     285554 | typer.rich_utils

@codecov

codecov Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.25%. Comparing base (a5aedbb) to head (3f62c98).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #887   +/-   ##
=======================================
  Coverage   85.25%   85.25%           
=======================================
  Files          51       51           
  Lines        4789     4789           
  Branches      712      712           
=======================================
  Hits         4083     4083           
  Misses        534      534           
  Partials      172      172           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@joanise

joanise commented Sep 16, 2026

Copy link
Copy Markdown
Member

Oops, you said here you added tests, but they're not there. Did you forget to commit them?

@joanise joanise left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@roedoejet
roedoejet merged commit 8e5c043 into main Oct 15, 2026
13 checks passed
@roedoejet
roedoejet deleted the dev.ap/fs2-loss branch October 15, 2026 16:01
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.

2 participants