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

Intel ARC / IPEX Docker support #3713

Open
wants to merge 15 commits into
base: dev
Choose a base branch
from
Open

Conversation

kmscode
Copy link

@kmscode kmscode commented Jan 20, 2025

Description

  • Print out the docker boolean properly
  • "Add" support for IPEX supported python versions
  • Support downloading IPEX from the US server via "SD_IPEX_USE_US_SERVER" variable
    • downloading from the CN server failed for me
  • new Dockerfile.arc to create IPEX supporting docker image

Notes

IPEX supports 3.9-3.12:
https://pytorch-extension.intel.com/installation?platform=gpu&version=v2.5.10%2Bxpu&os=linux%2Fwsl2&package=pip

Environment and Testing

This was my first attempt into playing around with genai/image generation - not too familiar with all of the sampling methods, but tried a few and some differing results as can be seen in the screenshots below.

Some images images showing process/processing and gpu usage and one output file:
Screenshot 2025-01-20 141254
Screenshot 2025-01-20 141314
Screenshot 2025-01-20 141327
Screenshot 2025-01-20 145545
00006-PixArt-XL-2-512x512-anime style two ninjas fighting

@vladmandic
Copy link
Owner

vladmandic commented Jan 20, 2025

thanks for the pr, few notes below...

i'd be ok with:

  • expanding allowed python versions,
  • fix for docker status
  • switching default download location to us and use of env variable to use cn instead if needed

the rest?

  • pr should never include submodules: right now you have both
    extensions-builtin/sdnext-modernui and extensions-builtin/sdnext-modernui which MUST be excluded
  • pr should always target dev branch, never master
  • inclusion of Dockerfile.arc in the main repo is not something i want to have
    we can have it in wiki or something?

cc @Disty0

@kmscode
Copy link
Author

kmscode commented Jan 21, 2025

@vladmandic Thanks for the feedback!

I'll fix the submodules. It seems they got pulled in when I did the latest merge from main best I Can tell.

The CONTRIBUTING file was a bit unclear - it said to fork from main, so I assumed pr should also go to main.

I was planning on updating the wiki as well, so I'll drop the Dockerfile.arc there.

Expect an update.

@kmscode kmscode changed the base branch from master to dev January 21, 2025 00:41
@vladmandic
Copy link
Owner

@vladmandic Thanks for the feedback!

I'll fix the submodules. It seems they got pulled in when I did the latest merge from main best I Can tell.

The CONTRIBUTING file was a bit unclear - it said to fork from main, so I assumed pr should also go to main.

I was planning on updating the wiki as well, so I'll drop the Dockerfile.arc there.

Expect an update.

ah, contribution file needs an update - thanks for pointing in out.

@kmscode
Copy link
Author

kmscode commented Jan 21, 2025

@vladmandic this should be good for further review now.

I added a quick note to CONTRIBUTING as well.

I had some edits/updates for the wiki in progress, I'll get those completed and include the updated ARC specific docker file there as well. That may be a few more days.

cc @Disty0

@Disty0
Copy link
Collaborator

Disty0 commented Jan 21, 2025

switching default download location to us and use of env variable to use cn instead if needed

Only reason we are using the CN location is because US doesn't work right now. US server is broken on Intel's side and throws 403 errors when trying to download ipex 2.5.

We can simply switch back to US if this issue is fixed on Intel's side.

@Disty0
Copy link
Collaborator

Disty0 commented Jan 21, 2025

Also i do have my dockerfile that only has the bare minimum for ipex if you are interested: https://github.com/Disty0/docker-sdnext-ipex/blob/main/Dockerfile

@vladmandic
Copy link
Owner

US server seems to be working now - can you double-check?
if that's the case, i'd rather have US as default instead of CN.

@Disty0
Copy link
Collaborator

Disty0 commented Jan 21, 2025

US server seems to be working now - can you double-check? if that's the case, i'd rather have US as default instead of CN.

I agree with US being the default, download speeds of CN server is terrible.

Seems like Intel updated their installation guide to use US server again but main issue on the ipex repo is still open: intel/intel-extension-for-pytorch#745

US server seems to be working fine on my end too, we can set US as the default or just switch back to US instead of adding a new env variable.
Users can use TORCH_COMMAND if they want to change it.

@vladmandic
Copy link
Owner

US server seems to be working fine on my end too, we can set US as the default.

lets do that. no need for CN-specific env variable as users can use TORCH_COMMAND env variable already

also, lets not edit CONTRIBUTING in this PR, its unrelated plus i want to make larger changes there.

@vladmandic
Copy link
Owner

@kmscode thanks for updates
@Disty0 pls take a look and if you're ok with this, merge

@kmscode
Copy link
Author

kmscode commented Jan 23, 2025

@vladmandic got this PR down to the bare minimum per the conversion notes above. Should be ready for review again.

@Disty0
Copy link
Collaborator

Disty0 commented Jan 24, 2025

Please also revert the Python version changes. It shouldn't log 3.9-3.12. Seeing 3.12 instead of 3.9, 3.10, 3.11 will lead people to install 3.12 and that will cause more compatibility issues.

@vladmandic
Copy link
Owner

true. python 3.12.3 works with cuda, but anything newer than that breaks pydantic and tons of other totally unrelated packages.

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