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

bugfix: Allow for optional TimeSlicing configuration #1018

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+12 −12
Open

bugfix: Allow for optional TimeSlicing configuration #1018

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e5f49f9
bugfix: Allow for optional timeslicing configuration
danielkleinstein Oct 27, 2024
14c6485
config: Remove unnecessary nil checks
danielkleinstein Oct 28, 2024
72b9e0b
resource: Fix resourceLabeler's sharingDisabled to check if a strateg…
danielkleinstein Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/config/v1/sharing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package v1
// Sharing encapsulates the set of sharing strategies that are supported.
type Sharing struct {
// TimeSlicing defines the set of replicas to be made for timeSlicing available resources.
TimeSlicing ReplicatedResources `json:"timeSlicing,omitempty" yaml:"timeSlicing,omitempty"`
TimeSlicing *ReplicatedResources `json:"timeSlicing,omitempty" yaml:"timeSlicing,omitempty"`
// MPS defines the set of replicas to be shared using MPS
MPS *ReplicatedResources `json:"mps,omitempty" yaml:"mps,omitempty"`
}
Expand Down Expand Up @@ -49,5 +49,5 @@ func (s *Sharing) ReplicatedResources() *ReplicatedResources {
if s.MPS != nil {
return s.MPS
}
return &s.TimeSlicing
return s.TimeSlicing
}
2 changes: 1 addition & 1 deletion internal/lm/mig-strategy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func TestMigStrategyNoneLabels(t *testing.T) {
},
},
Sharing: spec.Sharing{
TimeSlicing: tc.timeSlicing,
TimeSlicing: &tc.timeSlicing,
},
}

Expand Down
2 changes: 1 addition & 1 deletion internal/lm/nvml_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func TestSharingLabeler(t *testing.T) {
description: "config with timeslicing replicas",
config: &spec.Config{
Sharing: spec.Sharing{
TimeSlicing: spec.ReplicatedResources{
TimeSlicing: &spec.ReplicatedResources{
Resources: []spec.ReplicatedResource{
{
Replicas: 2,
Expand Down
2 changes: 1 addition & 1 deletion internal/lm/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ func (rl resourceLabeler) getReplicas() int {
// sharingDisabled checks whether the resourceLabeler has sharing disabled
// TODO: The nil check here is because we call NewGPUResourceLabeler with a nil config when sharing is disabled.
func (rl resourceLabeler) sharingDisabled() bool {
return rl.sharing == nil
return rl.sharing == nil || (rl.sharing.SharingStrategy() == spec.SharingStrategyNone)
}

// isShared checks whether the resource is shared.
Expand Down
8 changes: 4 additions & 4 deletions internal/lm/resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestGPUResourceLabeler(t *testing.T) {
description: "time-slicing ignores non-matching resource",
count: 1,
sharing: spec.Sharing{
TimeSlicing: spec.ReplicatedResources{
TimeSlicing: &spec.ReplicatedResources{
Resources: []spec.ReplicatedResource{
{
Name: "nvidia.com/not-gpu",
Expand All @@ -79,7 +79,7 @@ func TestGPUResourceLabeler(t *testing.T) {
description: "time-slicing appends suffix and doubles count",
count: 1,
sharing: spec.Sharing{
TimeSlicing: spec.ReplicatedResources{
TimeSlicing: &spec.ReplicatedResources{
Resources: []spec.ReplicatedResource{
{
Name: "nvidia.com/gpu",
Expand All @@ -103,7 +103,7 @@ func TestGPUResourceLabeler(t *testing.T) {
description: "time-slicing renamed does not append suffix and doubles count",
count: 1,
sharing: spec.Sharing{
TimeSlicing: spec.ReplicatedResources{
TimeSlicing: &spec.ReplicatedResources{
Resources: []spec.ReplicatedResource{
{
Name: "nvidia.com/gpu",
Expand Down Expand Up @@ -422,7 +422,7 @@ func TestMigResourceLabeler(t *testing.T) {
t.Run(tc.description, func(t *testing.T) {
config := &spec.Config{
Sharing: spec.Sharing{
TimeSlicing: tc.timeSlicing,
TimeSlicing: &tc.timeSlicing,
},
}
l, err := NewMIGResourceLabeler(tc.resourceName, config, device, tc.count)
Expand Down
6 changes: 3 additions & 3 deletions internal/rm/rm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestValidateRequest(t *testing.T) {
{
description: "timeslicing with single device",
sharing: spec.Sharing{
TimeSlicing: spec.ReplicatedResources{
TimeSlicing: &spec.ReplicatedResources{
Resources: []spec.ReplicatedResource{
{
Name: "nvidia.com/gpu",
Expand All @@ -73,7 +73,7 @@ func TestValidateRequest(t *testing.T) {
{
description: "timeslicing with two devices",
sharing: spec.Sharing{
TimeSlicing: spec.ReplicatedResources{
TimeSlicing: &spec.ReplicatedResources{
Resources: []spec.ReplicatedResource{
{
Name: "nvidia.com/gpu",
Expand All @@ -93,7 +93,7 @@ func TestValidateRequest(t *testing.T) {
{
description: "timeslicing with two devices -- failRequestsGreaterThanOne",
sharing: spec.Sharing{
TimeSlicing: spec.ReplicatedResources{
TimeSlicing: &spec.ReplicatedResources{
FailRequestsGreaterThanOne: true,
Resources: []spec.ReplicatedResource{
{
Expand Down