Repository navigation
Conversation
…g back to upstream or the CPU build
# Conflicts: # .github/workflows/unsloth-sd-prebuilt.yml
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6aa1e903c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| cp -r "$ROCM_PATH/lib/hipblaslt/library" "$BIN/hipblaslt/" | ||
| fi | ||
| for f in "$BIN"/sd-cli "$BIN"/sd-server "$BIN"/*.so*; do | ||
| [ -f "$f" ] && patchelf --set-rpath '$ORIGIN' "$f" |
There was a problem hiding this comment.
On this Ubuntu runner, this invokes the stock patchelf installed by apt above on every copied TheRock shared object. TheRock documents that released patchelf versions through 0.18.0 corrupt the PHDR of kpack-split ELF libraries and specifically requires Ubuntu users to install its patched build. The following validation only greps for not found, so an ldd loader error caused by that corruption takes the false branch and the broken ZIP can still be uploaded; use the pinned patched tool or avoid rewriting the affected libraries.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not reproducible here. The PHDR bug needs patchelf to grow and relocate the program headers; every rewrite in this step shrinks the RPATH to $ORIGIN or $ORIGIN/lib, which patchelf does in place. With stock patchelf 0.18.0 on every library in the 7.14.0 libraries wheel, including the kpack-split ones whose PHDR is in a trailing LOAD (librocblas, librccl, librocsolver, ...), the PHDR address is unchanged, readelf reports no errors, and the patched librocblas dlopens and resolves its whole closure from the bundle.
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
oobabooga
changed the title
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
3 participants
Studio's MiniMax-H3 GGUF path asks this mirror for an sd-cli built for
rocm. The latest release has none: macOS, Linux CPU, Linux CUDA, Linux aarch64, Windows CPU. So the installer falls back to leejet's upstream ROCm build, which doesn't carry the H3 fixes in this repo, and when that binary doesn't list a GPU,video.pydrops to the CPU build.That's unslothai/unsloth#8814:
7900 XTX on CachyOS, H3 goes
installedtoload_failedwithin a second. A Discord report this week on the same card and distro is the other shape of it: H3 Q3_K fills 30 GB of RAM and 36 GB of swap while VRAM sits at 1.5 GB.What this adds
A
build-linux-rocmleg inunsloth-sd-prebuilt.yml, same rule as the CUDA leg:continue-on-error, not in the coverage gate, so it can never hold back the CPU, Apple and Vulkan assets. The Linux and Windows Vulkan legs this PR first added came to master with unslothai#22, so they were dropped when master was merged in.build-linux-rocmLinux-Ubuntu-24.04-x86_64-rocm-7.14.0build.yml, for 20 consumer targets (gfx1010 to gfx1201, including the gfx1150 to gfx1153 APUs), and ships the ROCm userspace it linked against: every librarylddresolves from the wheel tree, rocBLAS / hipBLASLt kernel trees, and the per-target.kpackarchives the kpack-split libraries load. The binaries get rpath$ORIGIN/lib, the libraries$ORIGIN, and the leg fails if anything is still unresolved with the wheel tree hidden. Upstream's zip doesn't bundle the runtime, so it only runs on a host with a matching ROCm, which is the #8814 failure.package_bundle.pygainsKEEP_LAYOUT=1, which ships the staged tree as is (sd-cli,sd-server,lib/,.kpack/); the other legs still land flat.src/model_manager.cpp: a device whose free-memory report exceeds its total was treated as Vulkan's budget underflow and given 0 bytes. On a ROCm integrated GPU (Strix Halo) ggml reports free memory from/proc/meminfoand total fromhipMemGetInfo, so free is above total and every allocation was refused: every render aborted. That rejection now applies to Vulkan only, as upstream did in fix: handle GPU memory reports and LLM encoding failures leejet/stable-diffusion.cpp#2020, and a failed LLM prompt encode returns an error instead of aborting (the same upstream change).Asset names are what Studio's
resolve_release_assetalready matches forrocm, so a ROCm host picks this up with no Studio change. Not here: Windows ROCm.Validation
lddgate passes; zip 1802 MiB (GitHub's per-asset limit is 2 GiB)/opt/rocmis the optionallibhsa-amd-aqlprofile64/dev/kfd,/dev/dri)model_manager.cppchange on gfx1151available 0.00 MB device)Not run: a discrete AMD GPU (only the gfx1151 APU was available).