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: Load new model version should not reload loaded existing model version(s) #7527

Merged
merged 4 commits into from
Aug 20, 2024

Conversation

kthui
Copy link
Contributor

@kthui kthui commented Aug 14, 2024

What does the PR do?

Add the following tests on model version reload:

  1. If a version is loaded and unmodified, then it should not be reloaded on the next load request.
  2. If a version is loaded but modified, then it should be reloaded on the next load request.
  3. If a version is not loaded and its model file is in the model directory, then it should be loaded on the next load request.
  4. If a version is not loaded nor its model file is in the model directory, then it should be loaded on the next load request.
  5. If a generic file is modified on the model directory, i.e. model_directory/common_dependency.py, then all loaded version(s) should be reloaded on the next load request.

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.

  • build
  • ci
  • docs
  • feat
  • fix
  • perf
  • refactor
  • revert
  • style
  • test

Related PRs:

triton-inference-server/core#388

Where should the reviewer start?

Start with the core PR.

Test plan:

The PR adds the test, see the Test plan on the core PR.

  • CI Pipeline ID: 17511909 17650341

Caveats:

N/A

Background

N/A

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

N/A

@kthui kthui added the PR: test Adding missing tests or correcting existing test label Aug 14, 2024
@kthui kthui force-pushed the jacky-load-new-model-version branch from aabe4ca to b17c984 Compare August 14, 2024 19:47
@kthui kthui requested review from rmccorm4 and GuanLuo August 16, 2024 17:52
@kthui kthui marked this pull request as ready for review August 16, 2024 17:56
GuanLuo
GuanLuo previously approved these changes Aug 16, 2024
Copy link
Collaborator

@rmccorm4 rmccorm4 left a comment

Choose a reason for hiding this comment

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

Non-blocking for this release, but curious about this comment: https://github.com/triton-inference-server/server/pull/7527/files#r1722556601

@kthui kthui requested a review from rmccorm4 August 20, 2024 19:02
@kthui kthui merged commit 5e39771 into main Aug 20, 2024
3 checks passed
@kthui kthui deleted the jacky-load-new-model-version branch August 20, 2024 19:22
kthui added a commit that referenced this pull request Aug 20, 2024
mc-nv pushed a commit that referenced this pull request Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: test Adding missing tests or correcting existing test
Development

Successfully merging this pull request may close these issues.

3 participants