Cloudwatch Agent + DCGM Integration for nvidia-inference test #665
Conversation
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
wwvela
reviewed
Aug 6, 2025
| BertInferenceImage string `flag:"bertInferenceImage" desc:"BERT inference container image"` | ||
| InferenceMode string `flag:"inferenceMode" desc:"Inference mode for BERT (throughput or latency)"` | ||
| GpuRequested int `flag:"gpuRequested" desc:"Number of GPUs required for inference"` | ||
| NodeType string `flag:"nodeType" desc:"Instance type for cluster nodes"` |
Contributor
There was a problem hiding this comment.
Does nodeType required in inference test? It is not used anywhere
Contributor
Author
There was a problem hiding this comment.
Yes! Will remove it
Comment on lines
+40
to
+43
| testConfig = TestConfig{ | ||
| InferenceMode: "throughput", | ||
| GpuRequested: 1, | ||
| } |
Contributor
There was a problem hiding this comment.
- Could you comment above about this is config default value?
- We should maintain all config default value here including
InferenceModeto keep the consistency
Comment on lines
58
to
69
| // Render CloudWatch Agent manifest with dynamic dimensions | ||
| renderedCloudWatchAgentManifest, err := manifests.RenderCloudWatchAgentManifest(testConfig.MetricDimensions) | ||
| if err != nil { | ||
| log.Printf("Warning: failed to render CloudWatch Agent manifest: %v", err) | ||
| } | ||
|
|
||
| manifestsList := [][]byte{ | ||
| manifests.NvidiaDevicePluginManifest, | ||
| } | ||
|
|
||
| if len(testConfig.MetricDimensions) > 0 { | ||
| manifestsList = append(manifestsList, manifests.DCGMExporterManifest, renderedCloudWatchAgentManifest) | ||
| } |
Contributor
There was a problem hiding this comment.
- manifest render should happened inside the
if len(testConfig.MetricDimensions) > 0 {}after the manifestsList. if the metric dimension not exist, there is no need to do manifest render right? - Can you also revised this in training here
Contributor
Author
There was a problem hiding this comment.
Made PR: #667 to revise in nvidia-training.
Contributor
|
The build failed. you might need to sync the branch to pick up the latest merged changes |
annapoornanarayan
force-pushed
the
nvidia-inference-cw-agent
branch
from
6ac5638 to
e167589
Compare
annapoornanarayan
force-pushed
the
nvidia-inference-cw-agent
branch
from
5ae0cf0 to
1ae622e
Compare
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.
Issue #, if available:
Description of changes:
[DO NOT MERGE until nvidia-training PR is merged] The tests will only pass after the original changes are applied.
This PR contains changes to the
nvidia-inferencetest to deploy dcgm and cloudwatch manifests when enabled by--metricDimensionsflag.It also has standardization for flag formatting and common functions for daemonset deployment similar to
nvidia-trainingBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.