[Serve][1/N] Add Serve accelerator config schemas - #63581
Conversation
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
…unit tests Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
50c9d2d to
6d6540c
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces AcceleratorConfig and TPUAcceleratorConfig to Ray Serve, enabling structured hardware accelerator specifications for deployments, with an initial focus on TPU support. The changes span configuration models, protobuf definitions, and the deployment API, including validation logic to enforce mutual exclusivity between accelerator_config and gang_scheduling_config. Review feedback highlights an opportunity to improve security and cross-version compatibility by using JSON serialization instead of cloudpickle for the new configuration fields. Additionally, it is recommended to standardize the error messages for mutual exclusivity checks across the API, deployment, and schema modules for better consistency.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Ryan O'Leary <113500783+ryanaoleary@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Ryan O'Leary <113500783+ryanaoleary@users.noreply.github.com>
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
| optional double rolling_update_percentage = 23; | ||
|
|
||
| // Serialized representation of AcceleratorConfig. | ||
| bytes accelerator_config = 24; |
There was a problem hiding this comment.
Let's prefer a typed AcceleratorConfig message with a oneof for subtypes rather than bytes.
| bytes accelerator_config = 24; | |
| AcceleratorConfig accelerator_config = 24; |
message AcceleratorConfig {
oneof config {
TPUAcceleratorConfig tpu = 1;
}
}
message TPUAcceleratorConfig {
string topology = 1;
string accelerator_version = 2;
int32 num_slices = 3;
optional int32 chips_per_vm = 4;
map<string, double> resources_per_bundle = 5;
}
There was a problem hiding this comment.
we may need to adjust how to encode/decode the proto field in config.py
There was a problem hiding this comment.
done in 14c0807, they're now proto messages and we encode it like serve_pb2.TPUAcceleratorConfig
…alidation and expanded unit tests Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
Signed-off-by: Ryan O'Leary <113500783+ryanaoleary@users.noreply.github.com>
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 70bdeae. Configure here.
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
|
This pull request has been automatically marked as stale because it has not had You can always ask for help on our discussion forum or Ray's public slack channel. If you'd like to keep this open, just leave any comment, and the stale label will be removed. |
|
From offline discussion we're favoring implementing ray-project/enhancements#66 so that The Going to close these PRs and follow-up on the above. |

Description
PR split from #63179 based on this comment: #63179 (review). This PR contains just the new
AcceleratorConfigschemas and the plumbing through@serve.deployment,Deployment.options,DeploymentSchemaand the serve proto. This PR should be merged first, since the other PR contains these same changes.Related issues
#57137
Additional information