Skip to content

Fix argument to rec_loss in (Variational)AutoencoderLoss (from Samsung AI Center Cambridge)#2902

Merged
TParcollet merged 1 commit intospeechbrain:developfrom
rogiervd:pyright-vae_loss-reduction
May 13, 2025
Merged

Fix argument to rec_loss in (Variational)AutoencoderLoss (from Samsung AI Center Cambridge)#2902
TParcollet merged 1 commit intospeechbrain:developfrom
rogiervd:pyright-vae_loss-reduction

Conversation

@rogiervd
Copy link
Copy Markdown
Contributor

@rogiervd rogiervd commented May 13, 2025 •
edited
Loading

What does this PR do?

This fixes what is seemingly a bug. This was flagged by Pyright (see #2901).

The protocol for rec_loss is not specified, but its default value for rec_loss is mseloss, which has a parameter reduction="mean". So I assume the value for reduction should be "none" instead of None.

Before submitting
  • Did you read the contributor guideline?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you make sure to update the documentation with your changes? (if necessary)
  • Did you write any new necessary tests? (not for typos and docs)
  • Did you verify new and existing tests pass locally with your changes?
  • Did you list all the breaking changes introduced by this pull request?
  • Does your code adhere to project-specific code style and conventions?

PR review

Reviewer checklist
  • Is this pull request ready for review? (if not, please submit in draft mode)
  • Check that all items from Before submitting are resolved
  • Make sure the title is self-explanatory and the description concisely explains the PR
  • Add labels and milestones (and optionally projects) to the PR so it can be classified
  • Confirm that the changes adhere to compatibility requirements (e.g., Python version, platform)
  • Review the self-review checklist to ensure the code is ready for review

Fix argument to rec_loss in (Variational)AutoencoderLoss
b5a47d4 
The default value is mseloss, which takes a string-valued reduction.
@rogiervd rogiervd force-pushed the pyright-vae_loss-reduction branch from 02d1f53 to b5a47d4 Compare May 13, 2025 16:12
TParcollet
TParcollet approved these changes May 13, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@TParcollet TParcollet left a comment

Choose a reason for hiding this comment

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

LGTM

@TParcollet TParcollet merged commit 70dc8f7 into speechbrain:develop May 13, 2025
5 checks passed
pplantinga pushed a commit to pplantinga/speechbrain that referenced this pull request Jun 2, 2025
@rogiervd @pplantinga
Fix argument to rec_loss in (Variational)AutoencoderLoss (from Samsun…
bcd8264 
…g AI Center Cambridge) (speechbrain#2902)

Co-authored-by: Rogier van Dalen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TParcollet TParcollet TParcollet approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rogiervd @TParcollet