Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fx] skip diffusers unitest if it is not installed #1799

Merged
merged 3 commits into from
Nov 8, 2022

Conversation

feifeibear
Copy link
Contributor

No description provided.

@super-dainiu
Copy link
Contributor

Should we remove diffusers from test_requirements.txt also? The CI has difficulties installing this.

@FrankLeeeee
Copy link
Contributor

Should we remove diffusers from test_requirements.txt also? The CI has difficulties installing this.

The standard operation procedure is that, when a developer wants to the do the test, he/she should do pip install -r test-requirenements.txt first before executing pytest in stead of making this test condititonal.

@FrankLeeeee FrankLeeeee closed this Nov 7, 2022
@FrankLeeeee FrankLeeeee reopened this Nov 7, 2022
@FrankLeeeee
Copy link
Contributor

Sorry accidentally clicked on the close with comment button.

@feifeibear
Copy link
Contributor Author

Let's set dependencies such as torchrec, diffuser, and triton as optional in the CAI unit test. In my opinion, our unit tests do not need to be coupled with them.

  1. These libraries' interface updates are not under our control.
  2. Unitests need these libraries are bonus optimizatin, and their absence does not actually affect the core functions.

@FrankLeeeee
Copy link
Contributor

Let's set dependencies such as torchrec, diffuser, and triton as optional in the CAI unit test. In my opinion, our unit tests do not need to be coupled with them.

  1. These libraries' interface updates are not under our control.
  2. Unitests need these libraries are bonus optimizatin, and their absence does not actually affect the core functions.

OK, this is fine. I can create another decorator to handle this case. Otherwise, every unit test file depending on these libraries will have redundant code

try:
     import xxx
     HAS_XXX = True
else:
     HAS_XXX = False

@feifeibear
Copy link
Contributor Author

why use else instead of except?

@feifeibear feifeibear merged commit 6fa71d6 into hpcaitech:main Nov 8, 2022
@feifeibear feifeibear deleted the fx/diff_test branch November 8, 2022 03:46
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.

3 participants