Skip to content

Add SeFi-Image pipeline - #14084

Open
jmliu206 wants to merge 5 commits into
huggingface:mainfrom
jmliu206:sefi-image-diffusers
Open

Add SeFi-Image pipeline#14084
jmliu206 wants to merge 5 commits into
huggingface:mainfrom
jmliu206:sefi-image-diffusers

Conversation

@jmliu206

@jmliu206 jmliu206 commented Jun 29, 2026

Copy link
Copy Markdown

What does this PR do?

Adds Diffusers support for SeFi-Image, including:

  • SeFiTransformer2DModel
  • SeFiPipeline
  • a conversion script for original SeFi checkpoints
  • API docs and ToC entries
  • fast model and pipeline tests

The pipeline supports Qwen3-VL prompt encoding, SeFi dual-time denoising, classifier-free guidance for non-turbo checkpoints, turbo step validation, and model CPU offload.

Converted checkpoints

The original SeFi-Image checkpoint repositories remain unchanged.

Review follow-up

  • Flattened the Flux2-based transformer and made conversion from the original backbone.* keys strict.
  • Removed the unsupported LoRA decorator and simplified the pipeline fallbacks, chat-template handling, rotary access, copied helpers, and VAE path selection.
  • Made the scheduler conversion explicit and fixed the callback timestep value.

Before submitting

Who can review?

Potential reviewers for pipelines/models: @yiyixuxu @dg845 @asomoza

@github-actions github-actions Bot added documentation Improvements or additions to documentation models tests utils pipelines size/L PR with diff > 200 LOC labels Jun 29, 2026
@jmliu206 jmliu206 mentioned this pull request Jun 29, 2026
2 tasks
@jmliu206
jmliu206 marked this pull request as ready for review June 29, 2026 09:00
@jmliu206

Copy link
Copy Markdown
Author

@yiyixuxu @dg845 @asomoza — SeFi-Image modular port following #14086, ready for review. Happy to adjust anything I missed.

@vladmandic

Copy link
Copy Markdown
Contributor

@yiyixuxu @sayakpaul @asomoza @dg845 gentle ping - this pr is waiting for review for 3 weeks now?
@jmliu206 please upload converted models, its not reasonable to expect user to download original model, run conversion offline and then load it

@sergereview sergereview Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤗 Serge says:

Solid, well-tested first cut of the SeFi-Image pipeline, but there are several deviations from the project's coding rules (no defensive code, no unused params, # Copied from for adapted helpers) and a structural question about how the Flux2 backbone is reused.

Structure / conventions

  • SeFiTransformer2DModel instantiates a full Flux2Transformer2DModel as self.backbone, mutates time_guidance_embed to nn.Identity(), and then reaches into its internals (x_embedder, context_embedder, pos_embed, transformer_blocks, norm_out, proj_out, the three modulation modules) to reimplement the forward. This diverges from the single-file / # Copied from model conventions in .ai/models.md. Consider importing the Flux2 block classes directly (with # Copied from where appropriate) instead of wrapping the whole model and deleting one of its submodules. As written, the checkpoint will also carry dead time_guidance_embed params until the nn.Identity() swap, and self.out_channels on the SeFi model is set but never used.
  • Several pipeline helpers (_prepare_text_ids, _prepare_latent_ids, _pack_latents, _unpatchify_latents) appear adapted from pipelines/flux2/pipeline_flux2.py but lack # Copied from headers, so make fix-copies won't keep them in sync. _prepare_text_ids also carries an unused t_coord parameter (never passed by any caller).

Defensive code (repo rule: no defensive/fallback code)

  • SeFiPipeline.__init__ re-applies X if X is None else default for semantic_channels, texture_vae_name, default_guidance_scale, default_num_inference_steps, text_encoder_hidden_layers, max_sequence_length even though the signature already supplies non-None defaults. This is dead code unless a caller explicitly passes None.
  • _build_chat_text wraps apply_chat_template in a try/except TypeError fallback, and _align_text_encoder_rotary_dtype uses try/except StopIteration plus hasattr/getattr chains — both are the kind of "just in case" fallbacks the guidelines ask to drop in favor of clear expected inputs / concise errors.

LoRA decorator without the mixin

  • SeFiTransformer2DModel.forward is decorated with @apply_lora_scale("joint_attention_kwargs"), but the class does not inherit PeftAdapterMixin, so scale_lora_layers/unscale_lora_layers have nothing to act on. Either add the mixin (and wire up LoRA loading) or drop the decorator so the intent isn't misleading.

Correctness question

  • _timesteps_and_sigmas indexes directly into scheduler.timesteps/scheduler.sigmas at their training resolution and the pipeline never calls scheduler.set_timesteps. Since FlowMatchEulerDiscreteScheduler defaults to shift=3.0, that scheduler-side shift compounds with the pipeline's own timestep_shift_alpha. Please confirm the converted scheduler config uses shift=1.0 (or otherwise document/justify the compounding), since the fast tests don't exercise real checkpoint schedules.

Minor

  • texture_vae_config_path has two branches returning the identical path (root / "vae" / "config.json"); collapse them.
  • _current_timestep is set to a sigma value (base_sigmas_schedule[i]) and handed to callback_on_step_end as the timestep argument — slightly misleading naming.

Tests are structured correctly against the current tester mixins and cover inference, turbo validation, CFG batching, and the rotary-dtype alignment path, which is good.

serge v0.1.0 · model: claude-opus-4-8 · 32 LLM turns · 34 tool calls · 209.8s · 2083251 in / 12464 out tokens

Comment thread src/diffusers/models/transformers/transformer_sefi.py Outdated
Comment thread src/diffusers/models/transformers/transformer_sefi.py Outdated
Comment thread src/diffusers/pipelines/sefi/pipeline_sefi.py Outdated
Comment thread src/diffusers/pipelines/sefi/pipeline_sefi.py Outdated
Comment thread src/diffusers/pipelines/sefi/pipeline_sefi.py Outdated
Comment thread src/diffusers/pipelines/sefi/pipeline_sefi.py Outdated
Comment thread src/diffusers/pipelines/sefi/pipeline_sefi.py
Comment thread scripts/convert_sefi_to_diffusers.py
@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@jmliu206

jmliu206 commented Aug 3, 2026

Copy link
Copy Markdown
Author

Thanks for the review. I addressed each inline point and replied in the corresponding thread.

I also uploaded the standalone Diffusers conversions for the remaining variants:

The original SeFi-Image checkpoint repositories were not modified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation models pipelines size/L PR with diff > 200 LOC tests utils

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants