fix(lium): rent full offer gpu_count to avoid split 400#45
Conversation
Lium returns HTTP 400 "Provider doesn't allow GPU splitting" when the rent body requests gpu_count=1 on multi-GPU non-splittable executors. InstanceSpec.gpu_count is a minimum-need filter; the rent body must use the selected offer's full capacity instead.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughLium provisioning now conditionally sends the selected offer’s GPU count instead of the instance specification minimum. A unit test verifies this for non-splittable executors. The concurrent seal test uses a longer freshness interval and matching fake-clock advancement. ChangesLium GPU provisioning
Seal race test stability
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/base/compute/lium.py`:
- Around line 188-189: Update _resolve_offer to enforce spec.gpu_count for both
selection paths: filter automatic candidates so candidate.gpu_count is at least
the requested count, and validate explicit offers before returning them,
rejecting undersized offers. Only include the validated GPU count in rent_body.
In `@tests/unit/test_compute_lium_client.py`:
- Around line 218-222: Update the GPU payload assertions in the positive offer
test to require body["gpu_count"] == multi.gpu_count (8). Remove the permissive
membership check that accepts None or an omitted field, while retaining the
existing rejection of the incorrect value 1 if still needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 62d20939-ff74-4f40-93ea-01e497517335
📒 Files selected for processing (2)
src/base/compute/lium.pytests/unit/test_compute_lium_client.py
| if selected.gpu_count and selected.gpu_count > 0: | ||
| rent_body["gpu_count"] = selected.gpu_count |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject or filter undersized offers before renting.
_resolve_offer does not enforce spec.gpu_count: automatic selection chooses the cheapest offer regardless of GPU count, and explicit offers are returned without validation. This block can therefore rent fewer GPUs than requested instead of failing or selecting a compatible offer.
Proposed fix
+ if selected.gpu_count < spec.gpu_count:
+ raise LiumError(
+ f"offer {selected.id} provides {selected.gpu_count} GPUs, "
+ f"but {spec.gpu_count} are required"
+ )
if selected.gpu_count and selected.gpu_count > 0:
rent_body["gpu_count"] = selected.gpu_countAlso filter automatically selected offers by candidate.gpu_count >= spec.gpu_count.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if selected.gpu_count and selected.gpu_count > 0: | |
| rent_body["gpu_count"] = selected.gpu_count | |
| if selected.gpu_count < spec.gpu_count: | |
| raise LiumError( | |
| f"offer {selected.id} provides {selected.gpu_count} GPUs, " | |
| f"but {spec.gpu_count} are required" | |
| ) | |
| if selected.gpu_count and selected.gpu_count > 0: | |
| rent_body["gpu_count"] = selected.gpu_count |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/base/compute/lium.py` around lines 188 - 189, Update _resolve_offer to
enforce spec.gpu_count for both selection paths: filter automatic candidates so
candidate.gpu_count is at least the requested count, and validate explicit
offers before returning them, rejecting undersized offers. Only include the
validated GPU count in rent_body.
| body = json.loads(routes["rent"].calls.last.request.content) | ||
| assert body.get("gpu_count") != 1 | ||
| assert body.get("gpu_count") in (8, None) | ||
| if "gpu_count" in body: | ||
| assert body["gpu_count"] == multi.gpu_count |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the full GPU count explicitly.
With multi.gpu_count == 8, accepting None allows an implementation that omits gpu_count entirely to pass. Assert the exact payload value for this positive offer.
Proposed fix
- assert body.get("gpu_count") != 1
- assert body.get("gpu_count") in (8, None)
- if "gpu_count" in body:
- assert body["gpu_count"] == multi.gpu_count
+ assert body["gpu_count"] == multi.gpu_count📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| body = json.loads(routes["rent"].calls.last.request.content) | |
| assert body.get("gpu_count") != 1 | |
| assert body.get("gpu_count") in (8, None) | |
| if "gpu_count" in body: | |
| assert body["gpu_count"] == multi.gpu_count | |
| assert body["gpu_count"] == multi.gpu_count |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/test_compute_lium_client.py` around lines 218 - 222, Update the
GPU payload assertions in the positive offer test to require body["gpu_count"]
== multi.gpu_count (8). Remove the permissive membership check that accepts None
or an omitted field, while retaining the existing rejection of the incorrect
value 1 if still needed.
MasterWeightsResponse validates expires_at against real datetime.now, so freshness_seconds=1 failed when concurrent seal latency exceeded 1s on CI. Use a longer FakeClock TTL and expire via clock.advance.
2 participants
Summary
provision()rent body so multi-GPU non-splittable executors no longer get HTTP 400Provider doesn't allow GPU splitting.InstanceSpec.gpu_countstays a minimum-need filter; the rent request now uses the selected offer's fullgpu_count.Changes
LiumClient.provisionsets rentgpu_countfromselected.gpu_count(full capacity), notspec.gpu_count.Test plan
uv run pytest tests/unit/test_compute_lium_client.py— 71 passedBASE_LIVE_PROVIDER_TESTS=1+ real Lium key): rent → RUNNING → SSHnvidia-smi→ DELETE;result=success,pods_remaining=0, balance delta ~$0.01CI
Notes
Summary by CodeRabbit
Bug Fixes
Tests