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: Unit Tests for triton {metrics, config, status} #66

Merged
merged 17 commits into from
May 30, 2024

Conversation

KrishnanPrash
Copy link
Contributor

This PR is raised to address Jira Ticket [DLIS-6264].

Unit tests have been added primarily within triton_cli/tests/test_cli.py for triton metrics, triton config, and triton status.

Brief overview of the tests:
For metrics:

  • From the test_models repository, mock_llm is loaded.
  • Individual inferences are performed on each model
  • triton metrics output checked to see if nv_inference_request_success reflects successful inference.

For config:

  • From the test_models repository, add_sub and mock_llm are loaded.
  • triton config -m model_name is called.
  • The name field of the returned json is cross-referenced with the original model_name

For status:

  • From the test_models repository, add_sub and mock_llm are loaded.
  • triton status is called and verifies from output that the models are live and ready.

Secondary Changes that were made while implementing the tests:

  1. Adding grpcio>=1.64.0 to pyproject.toml.
  2. Standardizing JSON outputs returned by triton_cli: Triton commands were previously returning keys in single quotes and boolean values as {True, False} instead of {true,false}. This leads to errors when the output is attempted to be parsed as a json.

pyproject.toml Outdated Show resolved Hide resolved
tests/test_cli.py Outdated Show resolved Hide resolved
tests/test_cli.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
def test_triton_status(self, model, setup_and_teardown):
pid = utils.run_server(repo=MODEL_REPO) # Import the Model
setup_and_teardown.pid = pid
utils.wait_for_server_ready()
Copy link
Collaborator

Choose a reason for hiding this comment

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

For this PR, this is good - don't change it here.

However, some future food for thought - these tests that do (1) server start + (2) utility command should probably be in test_e2e.py. For test_cli.py it would probably be nice to have some super simple mock server implementation that can be used to test each of these commands standalone (more unit test style).

@rmccorm4 rmccorm4 changed the title Unit Tests for triton {metrics, config, status} test: Unit Tests for triton {metrics, config, status} May 30, 2024
@KrishnanPrash KrishnanPrash merged commit 31c8e0c into main May 30, 2024
6 checks passed
@KrishnanPrash KrishnanPrash deleted the kprashanth-model-config-test branch May 30, 2024 22:31
@rmccorm4
Copy link
Collaborator

Nice job merging your first PR already! 🚀

KrishnanPrash added a commit that referenced this pull request Jul 12, 2024
* Psuedocode for the testing metrics

* Blank Check

* Fixing  dependency issue

* Skeleton Code

* Dummy Prompt for inference

* MVP for testing

* Updting  dependency

* MVP for testing

* MVP for testing

* Standardizing JSON outputs

* MVP for testing

* Removing unnecessary model parameter

* applying pre-commit linting changes and removing unused imports

* Making inference check stricter

* Removing  test_model from  and  for consistency

* Final commit

* Fix dependency issues
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.

2 participants