Fix: swap input_amplitude and target_amplitude in JukeboxLoss.forward

[Zeesejo]

Fixes #8820

Description

In JukeboxLoss.forward(), the variable names input_amplitude and target_amplitude were swapped:

• input_amplitude was computed from target (should be from input)
• target_amplitude was computed from input (should be from target)

This fix corrects the assignments to match semantic meaning and the standard forward(input, target) PyTorch convention.

Types of changes

• Non-breaking change (fix or new feature that would not break existing functionality).
• Breaking change (fix or new feature that would cause existing functionality to change).
• New tests added to cover the changes.
• Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
• Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
• In-line docstrings updated.
• Documentation updated, tested make html command in the docs/ folder.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6ad368f9-edc3-46b2-8fb7-fedb4bcf02d4

📥 Commits

Reviewing files that changed from the base of the PR and between d0efb62 and 4417d79.

📒 Files selected for processing (1)

• monai/losses/ssim_loss.py

✅ Files skipped from review due to trivial changes (1)

• monai/losses/ssim_loss.py

---

📝 Walkthrough

Walkthrough

Fixes two small, independent edits: (1) in monai/losses/spectral_loss.py, JukeboxLoss.forward() now computes input_amplitude = _get_fft_amplitude(input) and target_amplitude = _get_fft_amplitude(target) (previously swapped); the MSE call is unchanged. (2) in monai/losses/ssim_loss.py, example usage in the SSIMLoss.forward docstring was updated to print SSIMLoss(spatial_dims=N)(x, y) directly for 2D and 3D. No public APIs or exported declarations were modified.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	Changes are limited to the spectral_loss.py file with a targeted fix. The raw summary mentions ssim_loss.py docstring updates, which appear unrelated to the linked issue.	Remove the unrelated docstring changes to ssim_loss.py, or clarify their necessity and link to a separate issue if intentional.
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main fix: swapping swapped variables in JukeboxLoss.forward.
Description check	✅ Passed	The description matches the template structure, includes issue reference (#8820), clear explanation of the bug, types of changes, and marks relevant checklist items.
Linked Issues check	✅ Passed	The PR correctly addresses issue #8820 by swapping the arguments to _get_fft_amplitude to compute input_amplitude from input and target_amplitude from target, fully satisfying the bug fix requirements.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

Fixes a semantic bug in JukeboxLoss.forward() where input_amplitude and target_amplitude were computed from the wrong tensors, aligning the implementation with the standard PyTorch forward(input, target) convention.

Changes:

• Compute input_amplitude from input (instead of target).
• Compute target_amplitude from target (instead of input).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The existing unit tests for JukeboxLoss validate numeric output/shape, but they won’t fail if input/target amplitudes are swapped because MSE is symmetric. To prevent regressions of this specific semantics fix, consider adding a test that patches/spies on _get_fft_amplitude (e.g., via unittest.mock) and asserts it is called first with input and then with target inside forward().

[Copilot]

PR description indicates "In-line docstrings updated", but this diff only swaps the input_amplitude/target_amplitude assignments and doesn’t modify any docstrings/comments. Either update the PR checklist/description to match the actual change set or include the intended docstring update in this PR.

[Zeesejo]

Update: This PR now also includes a second commit (4417d79) that fixes a docstring bug in SSIMLoss.forward() (tracked in #8822).

The forward() docstring examples showed print(1-SSIMLoss()(x,y)), but since SSIMLoss already computes 1 - ssim_value internally, those examples would actually return the raw SSIM value (not the loss). This fix removes the incorrect 1- prefix from all three docstring print examples.