Fix dead code and logic error

[ytl0623]

Description

1. added missing SPADE test cases to LATENT_CNDM_TEST_CASES
2. fixed the SPADE branch in test_sample_intermediates

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]

📝 Walkthrough

Walkthrough

Added SPADEAutoencoderKL as a new latent autoencoder variant in latent diffusion inference tests. The LATENT_CNDM_TEST_CASES_DIFF_SHAPES matrix now includes a SPADEAutoencoderKL configuration paired with SPADEDiffusionModelUNet, conditioning embedding, input/output latent shapes, and label_nc. Tests were updated to construct SPADEAutoencoderKL where ae_model_type matches, adjust segmentation input shapes to label_nc, include SPADEAutoencoderKL in intermediate-enabled branches (save_intermediates, intermediate_steps), and route its intermediates through the existing inferer pathways used for SPADEDiffusionModelUNet. Numerical assertion style changed to use assert_close for a normal CDF comparison.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	Description covers key changes but lacks issue reference and detail. Missing 'Fixes #' section at start per template.	Add 'Fixes #' reference at the start to complete template compliance and provide more context on what issues are being resolved.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title accurately captures the main changes: fixing dead code and logic errors related to SPADE test case handling.

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

✨ Finishing touches

• 📝 Generate docstrings

---

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.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/inferers/test_controlnet_inferers.py (1)

851-860: Fix stage_2 selection in test_pred_shape.
The else currently binds to the SPADEAutoencoderKL check and can overwrite a SPADEDiffusionModelUNet selection.

🔧 Proposed fix

-        if dm_model_type == "SPADEDiffusionModelUNet":
-            stage_2 = SPADEDiffusionModelUNet(**stage_2_params)
-        if ae_model_type == "SPADEAutoencoderKL":
-            stage_1 = SPADEAutoencoderKL(**autoencoder_params)
-        else:
-            stage_2 = DiffusionModelUNet(**stage_2_params)
+        if dm_model_type == "SPADEDiffusionModelUNet":
+            stage_2 = SPADEDiffusionModelUNet(**stage_2_params)
+        else:
+            stage_2 = DiffusionModelUNet(**stage_2_params)
+        if ae_model_type == "SPADEAutoencoderKL":
+            stage_1 = SPADEAutoencoderKL(**autoencoder_params)

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/inferers/test_controlnet_inferers.py (1)

849-860: Missing autoencoder instantiation for non-SPADE cases.

stage_1 remains None for AutoencoderKL and VQVAE test cases. Line 860 will raise AttributeError.

🐛 Proposed fix

     stage_1 = None

     if dm_model_type == "SPADEDiffusionModelUNet":
         stage_2 = SPADEDiffusionModelUNet(**stage_2_params)
     else:
         stage_2 = DiffusionModelUNet(**stage_2_params)
+    if ae_model_type == "AutoencoderKL":
+        stage_1 = AutoencoderKL(**autoencoder_params)
+    if ae_model_type == "VQVAE":
+        stage_1 = VQVAE(**autoencoder_params)
     if ae_model_type == "SPADEAutoencoderKL":
         stage_1 = SPADEAutoencoderKL(**autoencoder_params)
     controlnet = ControlNet(**controlnet_params)

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/inferers/test_controlnet_inferers.py (1)

1334-1360: Bug: Missing seg for SPADEAutoencoderKL with regular DiffusionModelUNet.

The test case at lines 362-401 pairs SPADEAutoencoderKL with DiffusionModelUNet. Line 1334 only checks for SPADEDiffusionModelUNet, so this case takes the else branch and doesn't pass seg. SPADEAutoencoderKL.decode_stage_2_outputs() requires seg.

🐛 Proposed fix

-        if dm_model_type == "SPADEDiffusionModelUNet":
+        if dm_model_type == "SPADEDiffusionModelUNet" or ae_model_type == "SPADEAutoencoderKL":

🧹 Nitpick comments (4)

tests/inferers/test_controlnet_inferers.py (4)

1017-1043: Inconsistent SPADE handling in test_get_likelihoods.

The condition at line 1017 only checks for SPADEDiffusionModelUNet, but test_prediction_shape (line 808) and other tests check for both SPADE variants. If a future test case uses SPADEAutoencoderKL with a regular DiffusionModelUNet, this test will fail because seg won't be passed to the inferer.

Consider aligning this condition:

-        if dm_model_type == "SPADEDiffusionModelUNet":
+        if ae_model_type == "SPADEAutoencoderKL" or dm_model_type == "SPADEDiffusionModelUNet":

---

1087-1115: Same inconsistency as test_get_likelihoods.

Line 1087 should also check for SPADEAutoencoderKL to stay consistent with other tests.

---

1171-1201: Same inconsistency as above.

Line 1171 should also check for SPADEAutoencoderKL.

---

1253-1281: Same inconsistency as above.

Line 1253 should also check for SPADEAutoencoderKL.

[ericspod]

Thanks for the fix @ytl0623. I noticed that Coderabbit mentions a bug outside the diff range of the PR, could you have a look at that as well? I think the changes here are fine though.

[ytl0623]

I've fixed the bugs, thanks!