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

test: Test model_metrics config and document histogram buckets override #7752

Merged
merged 7 commits into from
Nov 6, 2024

Conversation

yinggeh
Copy link
Contributor

@yinggeh yinggeh commented Oct 29, 2024

What does the PR do?

Tests new model_metrics message in pbtxt. Add example to override histogram buckets per-family.

Checklist

  • PR title reflects the change and is of format <commit_type>: <Title>
  • Changes are described in the pull request.
  • Related issues are referenced.
  • Populated github labels field
  • Added test plan and verified test passes.
  • Verified that the PR passes existing CI.
  • Verified copyright is correct on all changed files.
  • Added succinct git squash message before merging ref.
  • All template sections are filled out.
  • Optional: Additional screenshots for behavior/output changes with before/after.

Commit Type:

Check the conventional commit type
box here and add the label to the github PR.

  • docs
  • test

Related PRs:

triton-inference-server/core#405
triton-inference-server/common#126

Where should the reviewer start?

Test plan:

L0_metrics

  • CI Pipeline ID:
    20106057

Caveats:

Background

Default histogram buckets does not satisfy all use cases.

@yinggeh yinggeh added PR: docs Documentation only changes PR: test Adding missing tests or correcting existing test labels Oct 29, 2024
@yinggeh yinggeh self-assigned this Oct 29, 2024
@yinggeh yinggeh changed the title test: Test and document model metrics configuration test: Test model_metrics config and document histogram override Oct 29, 2024
@yinggeh yinggeh changed the title test: Test model_metrics config and document histogram override test: Test model_metrics config and document histogram buckets override Oct 29, 2024
qa/L0_metrics/histogram_metrics_test.py Outdated Show resolved Hide resolved
qa/L0_metrics/test.sh Outdated Show resolved Hide resolved
@yinggeh yinggeh force-pushed the yinggeh-DLIS-7457-per-model-metric-config branch from 64a5623 to 65c736e Compare October 31, 2024 01:23
@yinggeh yinggeh force-pushed the yinggeh-DLIS-7457-per-model-metric-config branch from 65c736e to 4af437e Compare October 31, 2024 03:25
@yinggeh yinggeh requested a review from GuanLuo October 31, 2024 03:27
@yinggeh yinggeh requested a review from rmccorm4 November 4, 2024 20:01

SERVER_ARGS="--model-repository=${MODELDIR} --model-control-mode=explicit --load-model=${decoupled_model} --metrics-config histogram_latencies=true --log-verbose=1"
run_and_check_server
export OVERRIDE_BUCKETS="-1,0,1,2.5,+Inf"
Copy link
Contributor

Choose a reason for hiding this comment

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

You are still hard-coding the buckets value..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hard coded here because it is non-trivial work to convert a double in string to the displayed bucket "le" value in Prometheus.

0.00 -> le="0"
1.0100 -> le="1.01"
100 -> le="100"
000 -> le="0"
1. -> le="1"

@yinggeh yinggeh requested a review from GuanLuo November 5, 2024 01:20
@yinggeh yinggeh requested a review from rmccorm4 November 6, 2024 00:12
@yinggeh
Copy link
Contributor Author

yinggeh commented Nov 6, 2024

Verified CI has completed without introducing new failures. @rmccorm4 @GuanLuo

@yinggeh yinggeh merged commit 462f31f into main Nov 6, 2024
3 checks passed
@yinggeh yinggeh deleted the yinggeh-DLIS-7457-per-model-metric-config branch November 6, 2024 04:28
@yinggeh yinggeh restored the yinggeh-DLIS-7457-per-model-metric-config branch November 6, 2024 04:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: docs Documentation only changes PR: test Adding missing tests or correcting existing test
Development

Successfully merging this pull request may close these issues.

3 participants