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

[CI][fix] Sagemaker integration test cloudwatch metrics fix #1385

Merged
merged 1 commit into from
Dec 13, 2023
Merged

Conversation

sindhuvahinis
Copy link
Contributor

Description

Sagemaker metrics were not uploaded to cloud watch, because the github action inputs will be empty when the workflow is dispatched through schedule.

Changes in this PR:

  • Made run_benchmark an argument to sagemaker-endpoint-tests script.
  • Uploading metrics for MME also, (before it was only for SME)
  • Metric name format will be {name}-{engine_name}-{num_partitions}p-{instance_type}-metric, for example opt-1-3-b-deepspeed-2p-ml.g5.8xlarge-throughput

Test:

  • Tested MME and SME 1 test case in my ec2 machine.

outputs = predictor.predict(DEFAULT_PAYLOAD, target_model=model)
print(outputs)

if run_benchmark:
Copy link
Contributor

Choose a reason for hiding this comment

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

do we convert run_benchmark to a bool anywhere? this would run if the value is "true" or "false", right (string values)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good one, missed it. Handled it now.

@sindhuvahinis sindhuvahinis force-pushed the ci branch 2 times, most recently from 7563cc9 to a6c08bc Compare December 12, 2023 20:50
@sindhuvahinis sindhuvahinis force-pushed the ci branch 2 times, most recently from f09cbe0 to 70efc42 Compare December 12, 2023 21:46
@sindhuvahinis sindhuvahinis merged commit cbc2b1b into master Dec 13, 2023
12 checks passed
@sindhuvahinis sindhuvahinis deleted the ci branch December 13, 2023 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants