refactor unet_1d tests#13898
Conversation
|
@askserge could you please review the PR? (testing) |
There was a problem hiding this comment.
🤗 Serge says:
The migration to the mixin-based test structure is mostly correct, but there are issues with the output_shape property values.
Correctness
-
UNetRLModelTesterConfig.output_shapeis set to(4, 14, 1)but the RL value-function model actually outputs(batch_size, 1)— i.e.(4, 1)for batch_size=4. The old test explicitly checkedtorch.Size((inputs_dict["sample"].shape[0], 1)). This is wrong regardless of whether the convention is per-sample or full-batch shape. It doesn't cause failures today because the basetest_outputhas an operator-precedence bug (== … or self.output_shapeis always truthy), and the customtest_outputoverride doesn't useself.output_shape. But it will break when the base class is fixed, and it's misleading to readers. -
UNet1DModelTesterConfig.output_shapeis(4, 14, 16)(includes batch dim), while the repo convention in other migrated tests (e.g.AutoencoderKLTesterConfig) is to use per-sample shape without the batch dimension —(3, 32, 32)not(4, 3, 32, 32). Should be(14, 16)to match the convention.
Minor
- In
TestUNetRLModel.test_output(line 173),self.get_dummy_inputs()is called a second time just to readshape[0](which is the hardcodedbatch_size = 4). This allocates a full set of tensors unnecessarily.
model: claude-opus-4-6 · 28 LLM turns · 30 tool calls · 143.1s · 627103 in / 6681 out tokens
| @property | ||
| def output_shape(self): | ||
| def output_shape(self) -> tuple: | ||
| return (4, 14, 16) |
There was a problem hiding this comment.
output_shape includes the batch dimension (4, 14, 16), but the repo convention (see e.g. AutoencoderKLTesterConfig) is to use the per-sample shape without the batch dim. Should be (14, 16) to stay consistent.
| return (4, 14, 16) | |
| return (14, 16) |
| @property | ||
| def output_shape(self): | ||
| def output_shape(self) -> tuple: | ||
| return (4, 14, 1) |
There was a problem hiding this comment.
output_shape is (4, 14, 1) but the RL value-function model outputs shape (batch_size, 1). The old test explicitly asserted torch.Size((batch_size, 1)). This value is wrong regardless of convention (per-sample it should be (1,), full-batch it should be (4, 1)).
The base test_output currently has an operator-precedence bug that makes the assertion vacuously true, and the custom test_output override doesn't reference self.output_shape, so this doesn't fail today — but it will break when the base class is fixed.
| return (4, 14, 1) | |
| return (1,) |
| with torch.no_grad(): | ||
| output = model(**self.get_dummy_inputs()).sample | ||
|
|
||
| assert output.shape == (self.get_dummy_inputs()["sample"].shape[0], 1), "Input and output shapes do not match" |
There was a problem hiding this comment.
Nit: self.get_dummy_inputs() is called a second time just to read shape[0], which is always the hardcoded batch_size = 4. This unnecessarily allocates a full set of tensors. Consider storing the inputs in a local variable:
| assert output.shape == (self.get_dummy_inputs()["sample"].shape[0], 1), "Input and output shapes do not match" | |
| inputs = self.get_dummy_inputs() | |
| assert output.shape == (inputs["sample"].shape[0], 1), "Input and output shapes do not match" |
(and pass inputs to the model call above as well)
* refactor unet_1d tests * use per-sample output_shape for unet_1d tests --------- Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>
* update * update * update * update * [CI] Refactor SD3 Transformer Test (#13340) * update * update --------- Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * refactor unet tests (3d_condition, motion, controlnetxs) (#13897) * refactor unet_3d_condition tests * refactor unet_motion tests * refactor unet_controlnetxs tests * refactor unet_1d tests (#13898) * refactor unet_1d tests * use per-sample output_shape for unet_1d tests --------- Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * refactor unet_2d tests (#13901) Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * [chore] log quant config to the user_agent (#13850) log quant config to the user_agent * Integrate AutoRound into Diffusers (#13552) * support auto_round Signed-off-by: Xin He <xin3.he@intel.com> * add document and unit tests Signed-off-by: Xin He <xin3.he@intel.com> * fix CI Signed-off-by: Xin He <xin3.he@intel.com> * Apply suggestions from code review Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com> * update document and overwrite the default quantization_config with specified backend. Signed-off-by: Xin He <xin3.he@intel.com> * add UT and fix bug Signed-off-by: Xin He <xin3.he@intel.com> * update per comments Signed-off-by: Xin He <xin3.he@intel.com> * update per comments Signed-off-by: Xin He <xin3.he@intel.com> * fix compile error in doc Signed-off-by: Xin He <xin3.he@intel.com> * Apply style fixes * small nits * Add auto_round dependency to the versions table Signed-off-by: Xin He <xin3.he@intel.com> * fix make deps_table_check_updated Signed-off-by: Xin He <xin3.he@intel.com> * fix CI Signed-off-by: Xin He <xin3.he@intel.com> --------- Signed-off-by: Xin He <xin3.he@intel.com> Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * [tests] refactor UNet model tests to align with the new pattern (#13153) * refactor unet2d condition model tests. * fix tests * up * fix * Revert "fix" This reverts commit 46d44b7. * up * recompile limit * [tests] refactor test_models_unet_1d.py to use modular testing mixins Refactor UNet1D model tests to follow the modern testing pattern using BaseModelTesterConfig and focused mixin classes (ModelTesterMixin, MemoryTesterMixin, TrainingTesterMixin, LoraTesterMixin). Both UNet1D standard and RL variants now have separate config classes and dedicated test classes organized by concern (core, memory, training, LoRA, hub loading). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * [tests] refactor test_models_unet_2d.py to use modular testing mixins Refactor UNet2D model tests (standard, LDM, NCSN++) to follow the modern testing pattern. Each variant gets its own config class and dedicated test classes organized by concern (core, memory, training, LoRA, hub loading). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * [tests] refactor test_models_unet_3d_condition.py to use modular testing mixins Refactor UNet3DConditionModel tests to follow the modern testing pattern with separate classes for core, attention, memory, training, and LoRA. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * [tests] refactor test_models_unet_controlnetxs.py to use modular testing mixins Refactor UNetControlNetXSModel tests to follow the modern testing pattern with separate classes for core, memory, training, and LoRA. Specialized tests (from_unet, freeze_unet, forward_no_control, time_embedding_mixing) remain in the core test class. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * [tests] refactor test_models_unet_spatiotemporal.py to use modular testing mixins Refactored the spatiotemporal UNet test file to follow the modern modular testing pattern with BaseModelTesterConfig and focused test classes: - UNetSpatioTemporalTesterConfig: Base configuration with model setup - TestUNetSpatioTemporal: Core model tests (ModelTesterMixin, UNetTesterMixin) - TestUNetSpatioTemporalAttention: Attention-related tests (AttentionTesterMixin) - TestUNetSpatioTemporalMemory: Memory/offloading tests (MemoryTesterMixin) - TestUNetSpatioTemporalTraining: Training tests (TrainingTesterMixin) - TestUNetSpatioTemporalLoRA: LoRA adapter tests (LoraTesterMixin) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * remove test suites that are passed. * fix consistencydecodervae tests * Revert "fix consistencydecodervae tests" This reverts commit 41b036b. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: dg845 <58458699+dg845@users.noreply.github.com> * [tests] fix vidtok tests (#13894) * fix vidtok tests * style * Update tests/models/autoencoders/test_models_autoencoder_vidtok.py Co-authored-by: dg845 <58458699+dg845@users.noreply.github.com> * Apply style fixes --------- Co-authored-by: dg845 <58458699+dg845@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * clean up --------- Signed-off-by: Xin He <xin3.he@intel.com> Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> Co-authored-by: Akshan Krithick <97239696+akshan-main@users.noreply.github.com> Co-authored-by: Xin He <xin3.he@intel.com> Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: dg845 <58458699+dg845@users.noreply.github.com>
What does this PR do?
Part of the ongoing modeling-test migration (following #13369 and #13153). Migrates the UNet1DModel test suites (the standard UNet and the RL value-function variant) to the mixin-based structure (Config + ModelTesterMixin).
UNet1DModel has no attention processors and doesn't support gradient checkpointing, so only ModelTesterMixin applies. The hub/pretrained integration tests are kept.
Before submitting
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
@sayakpaul @DN6