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: Add input byte size tests using C APIs #7372

Merged
merged 5 commits into from
Jul 3, 2024

Conversation

yinggeh
Copy link
Contributor

@yinggeh yinggeh commented Jun 25, 2024

What does the PR do?

Copies executable to qa/L0_input_validation directory and depended models to qa/L0_input_validation/models.

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:

  • test

Related PRs:

triton-inference-server/core#374

Where should the reviewer start?

N/A

Test plan:

  • CI Pipeline ID:
    16290532

Caveats:

N/A

Background

  1. Adding server level input byte size checks (TBD) will invalidate current tests in L0_input_validation since the error returned from server will be different from core.
  2. Some sophisticated logics were added in infer_request.cc that requires granular C API tests. See fix: Fix various tests caused by cases byte size validation not handled properly core#364

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

N/A

@yinggeh yinggeh self-assigned this Jun 25, 2024
@yinggeh yinggeh changed the title test: Add input byte size tests build: Add input byte size tests Jun 25, 2024
@yinggeh yinggeh added PR: build Changes that affect the build system or external dependencies PR: test Adding missing tests or correcting existing test and removed PR: build Changes that affect the build system or external dependencies labels Jun 25, 2024
@yinggeh yinggeh changed the title build: Add input byte size tests test: Add input byte size tests Jun 25, 2024
@yinggeh yinggeh changed the title test: Add input byte size tests test: Add input byte size tests using C APIs Jun 25, 2024
@yinggeh yinggeh force-pushed the yinggeh-DLIS-6658-input-byte-size-tests branch from 1039879 to dd4f3a6 Compare June 25, 2024 16:18
GuanLuo
GuanLuo previously approved these changes Jun 26, 2024
@yinggeh
Copy link
Contributor Author

yinggeh commented Jun 28, 2024

@GuanLuo I failed to include the test binary input_byte_size_test in L0_input_validation. Can you kindly review and approve?

@yinggeh
Copy link
Contributor Author

yinggeh commented Jun 28, 2024

Updated CI Pipeline ID @GuanLuo

GuanLuo
GuanLuo previously approved these changes Jun 29, 2024
@yinggeh yinggeh merged commit 52bb23f into main Jul 3, 2024
3 checks passed
@yinggeh yinggeh deleted the yinggeh-DLIS-6658-input-byte-size-tests branch July 3, 2024 18:08
@kthui
Copy link
Contributor

kthui commented Jul 4, 2024

@yinggeh just wondering if you have tried building on cpu-only-py3-base?

I'm getting this error could be related to this change

CMake Error at test/CMakeLists.txt:681 (target_link_libraries):
  Target "input_byte_size_test" links to:
    CUDA::cudart
  but the target was not found.  Possible reasons include:
    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.

jobs #99919520

@yinggeh
Copy link
Contributor Author

yinggeh commented Jul 4, 2024

@kthui Looks like cpu-only-py3-base is skipped by default (pipeline ID 16290505). I can quickly fix it.

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