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

GPU serving setting #935

Merged
merged 1 commit into from
Feb 9, 2023

Conversation

cfchase
Copy link
Member

@cfchase cfchase commented Feb 8, 2023

Allow ServingRuntimes to specify allowed number of GPUs

Closes: #940

Description

I've added an annotation to the ServingRuntime specification in the servingruntimes-configmap.yaml

data:
  override-config: |
    apiVersion: serving.kserve.io/v1alpha1
    kind: ServingRuntime
    metadata:
      annotations:
        opendatahub.io/gpu-setting: '1' #same as notebook gpuSetting

This is meant as a stopgap feature until full support for GPUs is implemented and tested, but hopefully follows the same development flow for servingruntimes-configs without the annotation.

How Has This Been Tested?

Change redhat-ods-applications servingruntimes-config to add an annotation to the override servingruntime

opendatahub.io/gpu-setting: '1'
opendatahub.io/gpu-setting: '5'
opendatahub.io/gpu-setting: '0'
opendatahub.io/gpu-setting: 'hidden'
no annotation

Verify the gpu selection shows as expected and test .

Configure a model server is spawned with correct limits/requests/affinities

Verify any pods spawned for the model server contain the correct limits/requests/affinities

Tested on OCP 4.11 with GPU operator installed using live build quay.io/cfchase/rhods-operator-live-catalog:1.22.0-w7

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@cfchase cfchase force-pushed the gpu-serving branch 5 times, most recently from 5fb8d05 to 0c6e7df Compare February 9, 2023 02:12
Copy link
Member

@andrewballantyne andrewballantyne left a comment

Choose a reason for hiding this comment

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

Looks pretty good on face-value -- I'll need to test it tomorrow; maybe @lucferbux can test this in his morning.

I think due to the amount of conflicts we will have -- you should probably merge first and I can go back through and try to fix all my PRs. Otherwise we'll be sharing in the conflicts between my various PRs.

@andrewballantyne
Copy link
Member

@cfchase I see that this is a WIP -- hopefully it's reasonably done otherwise we're going to have a race of conflicts tomorrow.

@cfchase cfchase changed the title [WIP] Allow GPU serving setting GPU serving setting Feb 9, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress This PR is in WIP state label Feb 9, 2023
@cfchase
Copy link
Member Author

cfchase commented Feb 9, 2023

@andrewballantyne I should have removed the WIP last night after I finished testing. Removed it

@cfchase
Copy link
Member Author

cfchase commented Feb 9, 2023

@andrewballantyne nevermind.... WIP back on while I rebase off of yours.

@cfchase cfchase changed the title GPU serving setting [WIP] GPU serving setting Feb 9, 2023
@andrewballantyne andrewballantyne mentioned this pull request Feb 9, 2023
3 tasks
@cfchase cfchase changed the title [WIP] GPU serving setting GPU serving setting Feb 9, 2023
@openshift-ci openshift-ci bot added the lgtm label Feb 9, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 9, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewballantyne

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved label Feb 9, 2023
@openshift-merge-robot openshift-merge-robot merged commit 0343596 into opendatahub-io:main Feb 9, 2023
lucferbux pushed a commit to red-hat-data-services/odh-dashboard that referenced this pull request Feb 13, 2023
strangiato pushed a commit to strangiato/odh-dashboard that referenced this pull request Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Add GPUs Tentatively For Model Serving Runtimes
3 participants