feat: support comma-separated gpu-scheduler-policy combinations - #2621
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughGPU scheduler policies now support comma-separated combinations. Shared membership checks update allocation behavior across GPU backends. Device sorting now applies ordered binpack, spread, and NUMA keys with deterministic tie-breaking. ChangesGPU policy composition
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Pod
participant PolicyParser
participant DeviceUsageList
participant DeviceBackend
Pod->>PolicyParser: provide comma-separated scheduler policies
PolicyParser->>DeviceUsageList: provide ordered sort keys
DeviceUsageList->>DeviceUsageList: compare binpack, spread, or NUMA keys
Pod->>DeviceBackend: provide scheduler policies
DeviceBackend->>PolicyParser: check mutex or topology membership
PolicyParser-->>DeviceBackend: return policy match
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
mesutoezdil
force-pushed
the
feat/gpu-scheduler-policy-combination
branch
from
August 12, 2026 10:34
c0a02ae to
0016dde
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@pkg/scheduler/policy/gpu_policy.go`:
- Around line 77-83: Update DeviceUsageList.Less to route a single "numa" policy
through lessByChain, while preserving the existing chained behavior for
comma-separated policies and legacy handling for other single policies. Add or
adjust the relevant gpu_policy tests with differing NUMA values and opposing
scores so the test distinguishes NUMA ordering from spread ordering.
- Around line 81-83: Update the combined-policy branch in the policy comparator
to preserve contiguous NUMA groups when NumaBind is enabled, applying NUMA-aware
ordering before chained score keys; retain lessByChain ordering for non-NumaBind
chains. Add coverage for a combined policy with NumaBind: true and ensure the
existing device-aware fit, score, and bind flow remains unchanged.
In `@pkg/util/util.go`:
- Line 292: Replace the strings.Split calls used by both range loops with
strings.SplitSeq, preserving the existing comma delimiter and loop behavior.
Update pkg/util/util.go lines 292-292 and pkg/scheduler/policy/gpu_policy.go
lines 65-65; both sites require the same direct change.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fe9baa1a-4b8c-43a3-a352-4da1f000fb91
📒 Files selected for processing (19)
pkg/device/amd/device.gopkg/device/ascend/device.gopkg/device/awsneuron/device.gopkg/device/biren/device.gopkg/device/cambricon/device.gopkg/device/enflame/device.gopkg/device/hygon/device.gopkg/device/iluvatar/device.gopkg/device/kunlun/vdevice.gopkg/device/metax/device.gopkg/device/metax/sdevice.gopkg/device/mthreads/device.gopkg/device/nvidia/device.gopkg/device/vastai/device.gopkg/scheduler/policy/gpu_policy.gopkg/scheduler/policy/gpu_policy_test.gopkg/util/types.gopkg/util/util.gopkg/util/util_test.go
mesutoezdil
force-pushed
the
feat/gpu-scheduler-policy-combination
branch
from
August 12, 2026 10:39
0016dde to
ee69b08
Compare
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
mesutoezdil
force-pushed
the
feat/gpu-scheduler-policy-combination
branch
from
August 12, 2026 10:44
ee69b08 to
4763bae
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
pkg/scheduler/policy/gpu_policy.go (1)
77-83: 🎯 Functional Correctness | 🟠 MajorRoute a single
numapolicy throughlessByChain.Line 81 only enables chained comparison when the policy contains a comma.
Policy: "numa"therefore uses the legacy spread comparator, which prioritizes score instead of NUMA. This violates the specified ordered sort-key behavior for a one-item policy chain.Add a regression case with opposing score and NUMA values.
Proposed fix
- if strings.Contains(l.Policy, ",") { + if strings.Contains(l.Policy, ",") || l.Policy == util.GPUSchedulerPolicyNuma.String() { return l.lessByChain(i, j) }🤖 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 `@pkg/scheduler/policy/gpu_policy.go` around lines 77 - 83, Update DeviceUsageList.Less to route the single "numa" policy through lessByChain, not the legacy comparator, while preserving chained-policy behavior. Add a regression test using opposing score and NUMA values to verify the NUMA ordering is selected for Policy: "numa".
🤖 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.
Duplicate comments:
In `@pkg/scheduler/policy/gpu_policy.go`:
- Around line 77-83: Update DeviceUsageList.Less to route the single "numa"
policy through lessByChain, not the legacy comparator, while preserving
chained-policy behavior. Add a regression test using opposing score and NUMA
values to verify the NUMA ordering is selected for Policy: "numa".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3ca41047-5b16-4f67-b2bf-e73385378a83
📒 Files selected for processing (2)
pkg/scheduler/policy/gpu_policy.gopkg/scheduler/policy/gpu_policy_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/scheduler/policy/gpu_policy_test.go
binpack/spread/numa chain as ordered sort keys, mutex/topology-aware stay Fit() filters. Single-value policies are unchanged. Adds PolicyContains to replace raw == comparisons across device backends. Closes #2010 Signed-off-by: mesutoezdil <114185853+mesutoezdil@users.noreply.github.com>
mesutoezdil
force-pushed
the
feat/gpu-scheduler-policy-combination
branch
from
August 12, 2026 11:02
4763bae to
b469f4b
Compare
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: archlitchi, mesutoezdil The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
KunwarSidhu47
pushed a commit
to KunwarSidhu47/HAMi
that referenced
this pull request
Aug 13, 2026
…ect-HAMi#2621) binpack/spread/numa chain as ordered sort keys, mutex/topology-aware stay Fit() filters. Single-value policies are unchanged. Adds PolicyContains to replace raw == comparisons across device backends. Closes Project-HAMi#2010 Signed-off-by: mesutoezdil <114185853+mesutoezdil@users.noreply.github.com> Signed-off-by: KunwarSidhu47 <kunwarsidhu1947@gmail.com>
This was referenced Aug 19, 2026
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
2 participants
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.
Lets gpu-scheduler-policy take an ordered comma-separated list.
binpack/spread/numa chain as sort keys in the order written, mutex/topology-aware stay filters in Fit().
Single values keep working as before.
Adds PolicyContains to replace the raw == checks that a comma would've silently broken.
I have tested all in my H100
Summary by CodeRabbit
New Features
Bug Fixes