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

Deduplicate ipex initialization code #1060

Merged
merged 2 commits into from
Jan 23, 2024
Merged

Conversation

akx
Copy link
Contributor

@akx akx commented Jan 18, 2024

This PR moves the repeated IPEX/XPU initialization code to a single place in library/ipex_interop.py.

cc @Disty0

fix vram usage in LoRA training
@Disty0
Copy link
Contributor

Disty0 commented Jan 19, 2024

  1. Don't change the init name to init_ipex and don't cahange the return values.
    I use my IPEX library with multiple projects. Unless you want to change them in every single project i use this library on, don't change them.

  2. I am not sure if ipex will work with importing it in a submodule instead of the main file.
    This needs testing when i get back to home.

  3. My purpose with using try / expect in the main file is that anything can happen in ipex libs (including straight up deleting it) and non ipex users will still be fine.
    This change makes it that if there is an issue with the ipex libs, entire script will error out.

@akx
Copy link
Contributor Author

akx commented Jan 19, 2024

Thanks for the comments!

Don't change the init name to init_ipex and don't cahange the return values. I use my IPEX library with multiple projects. Unless you want to change them in every single project i use this library on, don't change them.

Fair, I can change those back – you could consider packaging your library as an actual Python package, though. If you need a hand with that, please let me know.

I am not sure if ipex will work with importing it in a submodule instead of the main file.

It shouldn't have any technical difference. All this PR does is move the same code into a function and call it.

This change makes it that if there is an issue with the ipex libs, entire script will error out.

I don't think so. The exact same exception handling is in the new initialization function, although in a slightly different form. Anyway, I'll change this a little more to avoid touching library/ipex at all :)

library/ipex_interop.py Outdated Show resolved Hide resolved
@Disty0
Copy link
Contributor

Disty0 commented Jan 19, 2024

Other than the logging i mentioned, this pr is good to merge.

@kohya-ss
Copy link
Owner

Thank you for this PR and reviewing!

@kohya-ss kohya-ss changed the base branch from main to dev January 23, 2024 11:23
@kohya-ss kohya-ss merged commit bea4362 into kohya-ss:dev Jan 23, 2024
1 check passed
@akx akx deleted the refactor-xpu-init branch January 23, 2024 12:19
wkpark pushed a commit to wkpark/sd-scripts that referenced this pull request Feb 27, 2024
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