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.

Sign up for GitHub

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

Jump to bottom

(SD) Fix tokenizers imports in pyinstaller builds. #1828

Merged
merged 6 commits into from
Sep 12, 2023
Merged

(SD) Fix tokenizers imports in pyinstaller builds. #1828

merged 6 commits into from
Sep 12, 2023

Conversation

monorimet
Copy link
Collaborator

@monorimet monorimet commented Sep 11, 2023 •
edited
Loading

Also disables vae tuning for RDNA3.

gpetters94
gpetters94 reviewed Sep 12, 2023
View reviewed changes
Copy link
Contributor

@gpetters94 gpetters94 left a comment

Choose a reason for hiding this comment

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

LGTM

dan-garvey
dan-garvey reviewed Sep 12, 2023
View reviewed changes
Copy link
Member

@dan-garvey dan-garvey left a comment

Choose a reason for hiding this comment

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

why does it not matter that we get rid of gcs and opencv?

@monorimet
Copy link
Collaborator Author

monorimet commented Sep 12, 2023 •
edited
Loading

why does it not matter that we get rid of gcs and opencv?

These packages never actually got included in .exe, I have seen the following for the last few .exe debug sessions:

see https://github.com/nod-ai/SHARK/actions/runs/6142135995/job/16663459666#step:6:1042

2498 WARNING: collect_data_files - skipping data collection for module 'opencv_python' as it is not a package.
2737 WARNING: collect_data_files - skipping data collection for module 'google_cloud_storage' as it is not a package.

meaning we can remove them as they've always been skipped (at least the shark_downloader doesn't seem to need gcs. I'm not sure about opencv but hasn't posed issues yet.)

@PhaneeshB
Copy link
Contributor

PhaneeshB commented Sep 12, 2023 •
edited
Loading

meaning we can remove them as they've always been skipped (at least the shark_downloader doesn't seem to need gcs. I'm not sure about opencv but hasn't posed issues yet.)

Not sure why exe is skipping opencv, iirc,
OpenCV is imported for one of the stencil modes
EDIT:
used for canny - here
used for openpose - here

@monorimet
Copy link
Collaborator Author

Thanks @PhaneeshB. We can include it by asking for cv2.

@monorimet monorimet mentioned this pull request Sep 12, 2023
Closed
@PhaneeshB PhaneeshB self-requested a review September 12, 2023 16:40
PhaneeshB
PhaneeshB approved these changes Sep 12, 2023
View reviewed changes
Copy link
Contributor

@PhaneeshB PhaneeshB left a comment

Choose a reason for hiding this comment

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

LGTM!

@monorimet monorimet merged commit 684943a into main Sep 12, 2023
@monorimet monorimet deleted the ean-tokenizers branch September 12, 2023 17:23
Shukla-Gaurav pushed a commit to Shukla-Gaurav/dSHARK that referenced this pull request Sep 13, 2023
@monorimet @Shukla-Gaurav
(SD) Fix tokenizers imports in pyinstaller builds. (nod-ai#1828)
bc269d2 
* Fix tokenizers metadata.

* (SD) Disable VAE lowering configs (rdna3) and add versioned tunings.

* Update sd_annotation.py

* (SD) Add cv2 to spec.

* Update stencil pipeline with the new img2img arg.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PhaneeshB PhaneeshB PhaneeshB approved these changes

@dan-garvey dan-garvey Awaiting requested review from dan-garvey

@gpetters94 gpetters94 Awaiting requested review from gpetters94

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@monorimet @PhaneeshB @gpetters94 @dan-garvey