Skip to content

Comments

refactor the env setup and install of fastvideo#309

Merged
SolitaryThinker merged 2 commits intomainfrom
refactor-pyproject
Apr 4, 2025
Merged

refactor the env setup and install of fastvideo#309
SolitaryThinker merged 2 commits intomainfrom
refactor-pyproject

Conversation

@PorridgeSwim
Copy link
Contributor

Support optional lint install.
Remove sta_attn dependency.
Debug env_setup.py (flash_attn should has wheel preinstalled).

@SolitaryThinker
Copy link
Collaborator

Thank you for the effort!
Quick question, is it not possible to completely remove the env_setup.sh script? why?

@SolitaryThinker SolitaryThinker self-requested a review April 2, 2025 02:29
@SolitaryThinker
Copy link
Collaborator

I don't like the use of scripts to install flash-attn or sta. I am going to take a look at whether we just put these two dependencies as optional under the .toml file. I will directly modify this PR and work on this.

@SolitaryThinker
Copy link
Collaborator

Okay, I checked xDit/xfuers, which depends on flash-attn package, and their instructions in README actually fails if you start with a clean conda env.

The best way forward is to add flash-attn instruction in our README if people want to install it and fall back to torch sdpa backend if flash-attn package is not available (already being done)

cc @jzhang38 @PorridgeSwim

@zhisbug
Copy link
Collaborator

zhisbug commented Apr 4, 2025

Okay, I checked xDit/xfuers, which depends on flash-attn package, and their instructions in README actually fails if you start with a clean conda env.

The best way forward is to add flash-attn instruction in our README if people want to install it and fall back to torch sdpa backend if flash-attn package is not available (already being done)

cc @jzhang38 @PorridgeSwim

agree. Let's do that then. To summarize:

  1. README should be explict in suggesting to pip install flash-attn
  2. The attention backend will check fa existence and warn if not existed
  3. if fa is not installed, fall back to use torch attention.

@jzhang38
Copy link
Collaborator

jzhang38 commented Apr 4, 2025

Yes, I will set the default backend to SDPA in my PR: #312

Copy link
Collaborator

@SolitaryThinker SolitaryThinker left a comment

Choose a reason for hiding this comment

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

merging to unblock ci/cd work

@SolitaryThinker SolitaryThinker merged commit 4324c1c into main Apr 4, 2025
3 of 7 checks passed
kevin314 pushed a commit to kevin314/FastVideo that referenced this pull request Apr 5, 2025
Co-authored-by: Will Lin <wlsaidhi@gmail.com>
@SolitaryThinker SolitaryThinker deleted the refactor-pyproject branch June 30, 2025 22:39
qimcis pushed a commit to qimcis/FastVideo that referenced this pull request Oct 30, 2025
Co-authored-by: Will Lin <wlsaidhi@gmail.com>
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.

4 participants