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

fix compatibility with peft and diffusers 0.26.1 #626

Merged
merged 7 commits into from
Feb 7, 2024

Conversation

chengzeyi
Copy link
Contributor

@chengzeyi chengzeyi commented Feb 5, 2024

  • Fix converter warning for LoraCompatibleLinear.
  • Fix import error for diffusers==0.26.1 which causes patch failure.
  • Change default log level to warning to make mistakes more noticeable.
  • Check if libs are available before apply patches.

return flow.nn.Linear
# Workaround for Linear and LoRACompatibleLinear.
if inspect.isclass(obj):
return obj
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个问题已经在 #621 中修复

Copy link
Contributor Author

@chengzeyi chengzeyi Feb 5, 2024

Choose a reason for hiding this comment

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

@strint 不行的,我这里转LoraCompatibleLinear还是会有问题,diffusers==0.25.1也会

不过也不是不能跑。而是会打很多warning出来,因为转失败了

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ccssu 来看看,感觉是两个问题

Copy link
Contributor

Choose a reason for hiding this comment

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

很多warning出来具体是? 我用 python examples/text_to_image_sdxl_lora.py diffusers.version='0.25.1' 测试没有很多warning出来

Copy link
Contributor Author

Choose a reason for hiding this comment

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

现在main分支上的没有warning是因为onediff里把log level设置成error了,这反而造成出现很多问题不好定位,比如和新版本的diffusers的兼容性问题,这次也把这个修了

Copy link
Contributor Author

@chengzeyi chengzeyi Feb 5, 2024

Choose a reason for hiding this comment

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

顺便建议把onediffx的diffusers的版本要求放宽一点,现在diffusers更新的还是比较快的,已经到0.26了

Copy link
Contributor

Choose a reason for hiding this comment

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

我也建议 warning 要报出来。
这里我的意思是 153 重复了。 我在现在main分支上测的时候设置了 log level 为 debug ,
image

这个 inspect.isclass(obj): 和main 下面代码重复(记得是修复了 Unsupported type: <class 'type'> e=TypeError(' 这种警告。)

@torch2oflow.register
def _(mod: type):
    if not is_need_mock(mod):
        return mod
    return proxy_class(mod)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好像是没有修复,我是今天刚拉的?难道是main分支今天有变动?

Copy link
Contributor

Choose a reason for hiding this comment

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

嗯是的,今天的改动,我刚刚 Update Branch,可以试试把 if inspect.isclass(obj): 注释掉实验下

Copy link
Contributor Author

@chengzeyi chengzeyi Feb 5, 2024

Choose a reason for hiding this comment

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

嗯是的,今天的改动,我刚刚 Update Branch,可以试试把 if inspect.isclass(obj): 注释掉实验下

👍刚才试了一下已经修复了,所以已经把这段删掉了

@@ -37,7 +37,7 @@ def configure_logging(self, name, level, log_dir=None, file_name=None):

# Create a console formatter and add it to a console handler
console_formatter = ColorFormatter(
fmt="%(levelname)s [%(asctime)s] - %(message)s", datefmt="%Y-%m-%d %H:%M:%S"
fmt="%(levelname)s [%(asctime)s] %(filename)s:%(lineno)d - %(message)s", datefmt="%Y-%m-%d %H:%M:%S"
Copy link
Contributor

Choose a reason for hiding this comment

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

filename 为None 应该没必要加

Copy link
Contributor Author

Choose a reason for hiding this comment

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

有看打印出来的日志,filename不为none,还是有意义的

@chengzeyi
Copy link
Contributor Author

CI 挂了,不知道是啥原因,神奇

@chengzeyi
Copy link
Contributor Author

哦,好像CI挂了和我这个PR没啥关系,所有PR都挂了😄

@strint
Copy link
Collaborator

strint commented Feb 5, 2024

哦,好像CI挂了和我这个PR没啥关系,所有PR都挂了😄

是的,报错是因为 oneflow 那边一个修复还没编译出来

@isidentical
Copy link
Contributor

does this mean we can use peft and multiple LoRAs (for slow inference mode)?

@strint
Copy link
Collaborator

strint commented Feb 6, 2024

does this mean we can use peft and multiple LoRAs (for slow inference mode)?

@isidentical for peft, you can try this example #530 PR 530 is not related to this PR.

This is the original version of peft load. @marigoold is working on a fast load on peft.

@isidentical
Copy link
Contributor

also before I forget, onediffx is still uninstalled with diffusers==0.26.1 even with this PR:

"diffusers>=0.24.0,<=0.25.1",

@strint
Copy link
Collaborator

strint commented Feb 7, 2024

also before I forget, onediffx is still uninstalled with diffusers==0.26.1 even with this PR:

"diffusers>=0.24.0,<=0.25.1",

@chengzeyi need to make a test with onediffx

@chengzeyi
Copy link
Contributor Author

also before I forget, onediffx is still uninstalled with diffusers==0.26.1 even with this PR:

"diffusers>=0.24.0,<=0.25.1",

@chengzeyi need to make a test with onediffx

已修改

@isidentical
Copy link
Contributor

@chengzeyi can you do 0.26.2 (latest version)? Instead of 0.26.1

@chengzeyi
Copy link
Contributor Author

@chengzeyi can you do 0.26.2 (latest version)? Instead of 0.26.1

Changed but I hope the requirement could be loosened.

@chengzeyi chengzeyi merged commit 4da2270 into main Feb 7, 2024
3 of 5 checks passed
@chengzeyi chengzeyi deleted the dev_fix_compatibility_with_diffusers branch February 7, 2024 03:29
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