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.

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

feat(operator): Included LLM spec to CRD #6234

Merged
merged 18 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
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
2,310 changes: 1,215 additions & 1,095 deletions apis/go/mlops/scheduler/scheduler.pb.go

Large diffs are not rendered by default.

12 changes: 11 additions & 1 deletion apis/mlops/scheduler/scheduler.proto
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,14 @@ message ModelSpec {
repeated string requirements = 4; // list of capabilities the server must satisfy to run this model
optional uint64 memoryBytes = 5; // Requested memory
optional string server = 6; // the particular model server to load the model. If unspecified will be chosen.
optional ExplainerSpec explainer = 7; // optional black box explainer details
repeated ParameterSpec parameters = 8; // parameters to load with model
optional ModelRuntimeInfo modelRuntimeInfo = 9; // model specific settings that are sent by the agent

// ensure only one of explainer or llm is specified at a time
oneof model_spec {
ExplainerSpec explainer = 7; // optional black box explainer details
LlmSpec llm = 10; // optional LLM specific settings
}
}

message ParameterSpec {
Expand All @@ -59,6 +64,11 @@ message ExplainerSpec {
optional string pipelineRef = 3;
}

message LlmSpec {
optional string modelRef = 1;
optional string pipelineRef = 2;
}

message ModelRuntimeInfo {
oneof modelRuntimeInfo {
MLServerModelSettings mlserver = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,18 @@ spec:
description: type of explainer
type: string
type: object
llm:
description: Llm spec
properties:
modelRef:
description: |-
one of the following need to be set for the llm
Reference to Model
type: string
pipelineRef:
description: Reference to Pipeline
type: string
type: object
logger:
description: Payload logging
properties:
Expand Down
12 changes: 12 additions & 0 deletions k8s/yaml/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,18 @@ spec:
description: type of explainer
type: string
type: object
llm:
description: Llm spec
properties:
modelRef:
description: |-
one of the following need to be set for the llm
Reference to Model
type: string
pipelineRef:
description: Reference to Pipeline
type: string
type: object
logger:
description: Payload logging
properties:
Expand Down
42 changes: 38 additions & 4 deletions operator/apis/mlops/v1alpha1/model_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ type ModelSpec struct {
Explainer *ExplainerSpec `json:"explainer,omitempty"`
// Parameters to load with model
Parameters []ParameterSpec `json:"parameters,omitempty"`
// Llm spec
Llm *LlmSpec `json:"llm,omitempty"`
}

func (m *ModelSpec) Validate() error {
if m.Explainer != nil && m.Llm != nil {
return fmt.Errorf("Can't have both explainer and llm in model spec.")
}
return nil
}

type ParameterSpec struct {
Expand All @@ -64,6 +73,17 @@ type ExplainerSpec struct {
PipelineRef *string `json:"pipelineRef,omitempty"`
}

// Either ModelRef or PipelineRef is required
type LlmSpec struct {
// one of the following need to be set for the llm
// Reference to Model
// +optional
ModelRef *string `json:"modelRef,omitempty"`
// Reference to Pipeline
// +optional
PipelineRef *string `json:"pipelineRef,omitempty"`
}

type ScalingSpec struct {
// Number of replicas - default 1
Replicas *int32 `json:"replicas,omitempty"`
Expand Down Expand Up @@ -143,6 +163,10 @@ func init() {

// Method to convert Model resource to scheduler proto for communication with Scheduler
func (m Model) AsSchedulerModel() (*scheduler.Model, error) {
// guarantees that Explainer and Llm are mutually exclusive
if err := m.Spec.Validate(); err != nil {
return nil, err
}
md := &scheduler.Model{
Meta: &scheduler.MetaData{
Name: m.Name,
Expand All @@ -162,10 +186,20 @@ func (m Model) AsSchedulerModel() (*scheduler.Model, error) {
},
}
if m.Spec.Explainer != nil {
md.ModelSpec.Explainer = &scheduler.ExplainerSpec{
Type: m.Spec.Explainer.Type,
ModelRef: m.Spec.Explainer.ModelRef,
PipelineRef: m.Spec.Explainer.PipelineRef,
md.ModelSpec.ModelSpec = &scheduler.ModelSpec_Explainer{
Explainer: &scheduler.ExplainerSpec{
Type: m.Spec.Explainer.Type,
ModelRef: m.Spec.Explainer.ModelRef,
PipelineRef: m.Spec.Explainer.PipelineRef,
},
}
}
if m.Spec.Llm != nil {
RobertSamoilescu marked this conversation as resolved.
Show resolved Hide resolved
md.ModelSpec.ModelSpec = &scheduler.ModelSpec_Llm{
Llm: &scheduler.LlmSpec{
ModelRef: m.Spec.Llm.ModelRef,
PipelineRef: m.Spec.Llm.PipelineRef,
},
}
}
if len(m.Spec.Parameters) > 0 {
Expand Down
131 changes: 126 additions & 5 deletions operator/apis/mlops/v1alpha1/model_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,55 @@ import (
scheduler "github.com/seldonio/seldon-core/apis/go/v2/mlops/scheduler"
)

func TestModelSpec_Validate(t *testing.T) {
g := NewGomegaWithT(t)
tests := []struct {
name string
spec *ModelSpec
wantErr bool
}{
{
name: "Both explainer and llm set",
spec: &ModelSpec{
Explainer: &ExplainerSpec{},
Llm: &LlmSpec{},
},
wantErr: true,
},
{
name: "Only explainer set",
spec: &ModelSpec{
Explainer: &ExplainerSpec{},
},
wantErr: false,
},
{
name: "Only llm set",
spec: &ModelSpec{
Llm: &LlmSpec{},
},
wantErr: false,
},
{
name: "Neither explainer nor llm set",
spec: &ModelSpec{},
wantErr: false,
},
}

for _, tt := range tests {
err := tt.spec.Validate()

if tt.wantErr {
g.Expect(err).ToNot(BeNil())
g.Expect(err.Error()).To(ContainSubstring("Can't have both explainer and llm in model spec."))
} else {
g.Expect(err).To(BeNil())
}
}

}

func TestAsModelDetails(t *testing.T) {
g := NewGomegaWithT(t)
type test struct {
Expand All @@ -39,7 +88,7 @@ func TestAsModelDetails(t *testing.T) {
server := "server"
m1 := resource.MustParse("1M")
m1bytes := uint64(1_000_000)
incomeModel := "income"
incomeModel, llmModel := "income", "chat-gpt"
tests := []test{
{
name: "simple",
Expand Down Expand Up @@ -75,7 +124,7 @@ func TestAsModelDetails(t *testing.T) {
},
},
{
name: "complex",
name: "complex - explainer",
model: &Model{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Expand Down Expand Up @@ -121,9 +170,81 @@ func TestAsModelDetails(t *testing.T) {
Requirements: []string{"a", "b", modelType},
StorageConfig: &scheduler.StorageConfig{Config: &scheduler.StorageConfig_StorageSecretName{StorageSecretName: secret}},
Server: &server,
Explainer: &scheduler.ExplainerSpec{
Type: "anchor_tabular",
ModelRef: &incomeModel,
ModelSpec: &scheduler.ModelSpec_Explainer{
Explainer: &scheduler.ExplainerSpec{
Type: "anchor_tabular",
ModelRef: &incomeModel,
},
},
Parameters: []*scheduler.ParameterSpec{
{
Name: "foo",
Value: "bar",
},
{
Name: "foo2",
Value: "bar2",
},
},
},
DeploymentSpec: &scheduler.DeploymentSpec{
Replicas: 4,
LogPayloads: true,
MinReplicas: 0,
MaxReplicas: 0,
},
},
},
{
name: "complex - llm",
model: &Model{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "default",
Generation: 1,
},
Spec: ModelSpec{
InferenceArtifactSpec: InferenceArtifactSpec{
ModelType: &modelType,
StorageURI: "gs://test",
SecretName: &secret,
},
Logger: &LoggingSpec{},
Requirements: []string{"a", "b"},
ScalingSpec: ScalingSpec{Replicas: &replicas},
Server: &server,
Llm: &LlmSpec{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think that we should have a test case specific to LLM spec and not have it with explainer. In fact the user should not be able to set the two together and this can should be marked as invalid perhaps.

ModelRef: &llmModel,
},
Parameters: []ParameterSpec{
{
Name: "foo",
Value: "bar",
},
{
Name: "foo2",
Value: "bar2",
},
},
},
},
modelpb: &scheduler.Model{
Meta: &scheduler.MetaData{
Name: "foo",
KubernetesMeta: &scheduler.KubernetesMeta{
Namespace: "default",
Generation: 1,
},
},
ModelSpec: &scheduler.ModelSpec{
Uri: "gs://test",
Requirements: []string{"a", "b", modelType},
StorageConfig: &scheduler.StorageConfig{Config: &scheduler.StorageConfig_StorageSecretName{StorageSecretName: secret}},
Server: &server,
ModelSpec: &scheduler.ModelSpec_Llm{
Llm: &scheduler.LlmSpec{
ModelRef: &llmModel,
},
},
Parameters: []*scheduler.ParameterSpec{
{
Expand Down
30 changes: 30 additions & 0 deletions operator/apis/mlops/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions operator/config/crd/bases/mlops.seldon.io_models.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,18 @@ spec:
description: type of explainer
type: string
type: object
llm:
description: Llm spec
properties:
modelRef:
description: |-
one of the following need to be set for the llm
Reference to Model
type: string
pipelineRef:
description: Reference to Pipeline
type: string
type: object
logger:
description: Payload logging
properties:
Expand Down
Loading
Loading