Skip to content

Update get_2d_sincos_pos_embed to use output_type='pt' for Diffusers compat#504

Merged
feifeibear merged 2 commits intoxdit-project:mainfrom
fielding:fix/get-2d-sincos-pos-embed-output-type
Apr 23, 2025
Merged

Update get_2d_sincos_pos_embed to use output_type='pt' for Diffusers compat#504
feifeibear merged 2 commits intoxdit-project:mainfrom
fielding:fix/get-2d-sincos-pos-embed-output-type

Conversation

@fielding
Copy link
Contributor

Description
This PR addresses a deprecation warning in Diffusers 0.33.0 related to the output_type parameter in get_2d_sincos_pos_embed. The function now recommends using output_type='pt' to return a PyTorch tensor directly, which is more efficient and avoids unnecessary conversions from NumPy.

Changes

  • Modified get_2d_sincos_pos_embed in xfuser/model_executor/layers/embeddings.py to use output_type='pt'.
  • Added the device parameter to ensure the tensor is placed directly on the correct device, removing the need for manual device transfer.
  • Removed the previous torch.from_numpy conversion and .to(latent.device) call, as they are no longer required these changes.

Benefits

  • Ensures xFuser remains compatible with Diffusers' updated API, avoiding future issues.
  • Improves performance by eliminating NumPy-to-PyTorch conversions and redundant device transfers.
  • Simplifies the code.

Testing

  • Tested with Diffusers 0.33.0.dev0 live on a 8x H200 machine. Can report back testings on a more variety of machines if needed.

Cheers

grid_size=(height, width),
base_size=self.module.base_size,
interpolation_scale=self.module.interpolation_scale,
device=latent.device, # Place tensor on the correct device directly
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this change work with lower versions of diffusers, such as 0.32.0? If it's only compatible with 0.33.0, we could implement an if-else statement with a diffuser version check as the condition."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooops, ignore the vanishing comments.

Good catch! I was in a bit of a hurry and didn't check when the arguments were added, haha. I confirmed it was 0.33.0, so I've updated it to use the simplified code conditionally.

Appreciate you pointing that out! Let me know if anything else is needed! Cheers!

@fielding fielding requested a review from feifeibear April 23, 2025 01:57
@feifeibear feifeibear merged commit 732ab13 into xdit-project:main Apr 23, 2025
fielding added a commit to fielding/xDiT that referenced this pull request May 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants