feat(cluster): report VRAM_MB booking in cluster status - #172
Draft
nilsmechtel wants to merge 2 commits into
Draft
feat(cluster): report VRAM_MB booking in cluster status#172nilsmechtel wants to merge 2 commits into
nilsmechtel wants to merge 2 commits into
Conversation
On a cluster advertising the VRAM_MB custom resource the AppBuilder books a 0.01 GPU handle per replica purely to bind a device, so used_gpu reads as near-idle on a GPU whose VRAM is fully reserved: four federated-unet replicas on Europa report used_gpu 0.04 while holding 20480 of 24576 MB. get_cluster_state now carries total_vram_mb/used_vram_mb per node and rolled up, and the CLI prints the booking next to the fraction on nodes that advertise it. Nodes without VRAM_MB report 0 and their output is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
get_statusreportsused_gpu: 0.04on a GPU that is 83% reserved. Fourfederated-unetreplicas are running onbioengine-worker-europa, each holding 5120 MB of a 24576 MB RTX 3090 — but the cluster block reads as an almost entirely idle GPU, which makes the status output actively misleading for anyone deciding whether another GPU app will fit.That number is not wrong, it is answering a different question. BioEngine apps declare
gpu_memory_mb, nevernum_gpus. On a cluster that advertises theVRAM_MBcustom resource,AppBuilder._apply_gpu_memorybooksnum_gpus = _GPU_HANDLE_EPSILON(0.01) purely to bind a device and populateCUDA_VISIBLE_DEVICES, and puts the real reservation inresources["VRAM_MB"]. Soused_gpuon such a cluster is a count of device handles — 4 replicas × 0.01 — and the quantity that actually bounds packing is invisible.This PR surfaces that quantity.
get_cluster_statenow carriestotal_vram_mb/used_vram_mbper node and, via the existing rollup loop, at cluster level. Both are derived the same way asused_cpu/used_gpu(total − available), and the_mbsuffix keeps them distinct from the byte-valued, NVML-measuredtotal_gpu_memory/used_gpu_memory, which report what the driver observes rather than what the scheduler has booked. The three numbers are genuinely different and the PR does not merge them.On Europa the new fields read
used_vram_mb: 20480 / total_vram_mb: 24576next toused_gpu: 0.04.The same value has a second exit path, so
bioengine cluster statusis updated too: it printedGPU: 0.0/1for that saturated GPU (.1fof 0.04). It now prints the fraction at.2fand appends the booking on nodes that advertiseVRAM_MB:Nodes with no
VRAM_MBresource report0for both fields and their CLI line is unchanged. The keys are always present rather than conditional, so table renderers overnodesdo not have to special-case their absence — but a0there means "this cluster does not use VRAM_MB booking", not "this GPU is free", and on those clusters the GPU fraction remains the real reservation.What is verified, and what is not
tests/test_cluster_status_vram.pydrives the plain class behind the Ray actor decorator with stubbed node resources, so the branch runs without a GPU cluster. Two tests: the Europa case (VRAM_MB advertised, booking reported where the GPU fraction is not), and the deNBI-shaped case (no VRAM_MB, both fields0,used_gpucarrying a real 0.33 fraction). A positive control was run — neuteringused_vram_mbto0fails the first test while the second still passes, so the pair discriminates rather than both passing for free.Two things are honestly not proven here:
No live-cluster validation of the VRAM_MB branch. It is only exercised on a cluster that advertises the resource. Europa is currently running the federated consortium (4 replicas, CPU 8/8) and rolling it needs explicit sign-off; a CPU-only e2e run would satisfy the new e2e assertions vacuously. The additions to
tests/end_to_end/test_worker.pyare guarded byif field in …like every field around them, so they type-check the new keys where present and pass silently where not.The new tests cannot run under pytest in this environment.
tests/conftest.pyhas a session-scoped autousevalidate_environmentfixture that walksrequirements-worker.txtand callspytest.exit()on the first package missing from the running interpreter.aiortc==1.14.0is declared (pyproject.toml:62,requirements-worker.txt:1) but not installed here, so any test inheriting that fixture exits before collection —tests/test_gpu_sizing.py, an existing neighbour of the new file, fails identically on a clean checkout. Under xdist this surfaces as a crashed worker rather than the exit message, which is worth knowing before debugging it as a test bug.This is not suite-wide:
tests/_app/conftest.py:11overrides the fixture with a no-op, andtests/_app/test_decorators.pyruns its 26 tests green here. The new file sits attests/top level like its neighbours and inherits the gate along with them, so it is left as-is rather than opting itself out of a deliberate check. Its two tests were executed standalone, bypassingconftest. Environment gap, not introduced by this PR, and CI installs the worker requirements so it is unaffected.Draft: no version bump yet, per the repo's bump-just-before-ready rule.
🤖 Generated with Claude Code