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

Decouple TritonInferenceStage from pipeline mode #1402

Merged
merged 58 commits into from
Jan 8, 2024

Conversation

dagardner-nv
Copy link
Contributor

@dagardner-nv dagardner-nv commented Dec 1, 2023

Description

  • Add needs_logits & inout_mapping a constructor arguments to TritonInferenceStage.
  • Consolidate triton inference worker impls into a single TritonInferenceWorker class
  • For compatibility the needs_logits & inout_mapping arguments default to default values inferred by the pipeline mode.
  • Refactor custom triton inference stage from examples/log_parsing example as a subclass of TritonInferenceStage removing several lines of redundant code.
  • examples/log_parsing example now works in C++ mode
  • Add the ability to subclass a stage registered with the CLI, and register the sublcass with a new CLI name.
  • Add the ability to parse dictionary types with click.
  • Remove inf-triton from the AE mode in the CLI
  • Misc linting/formatting pylint suggestions

Closes #1378

By Submitting this PR I confirm:

  • I am familiar with the Contributing Guidelines.
  • When the PR is ready for review, new or existing tests cover these changes.
  • When the PR is ready for review, the documentation is up to date with these changes.

@dagardner-nv dagardner-nv self-assigned this Dec 1, 2023
@dagardner-nv dagardner-nv requested a review from a team as a code owner December 1, 2023 17:14
@dagardner-nv dagardner-nv marked this pull request as draft December 1, 2023 17:14
@dagardner-nv dagardner-nv added breaking Breaking change improvement Improvement to existing functionality labels Dec 1, 2023
@dagardner-nv dagardner-nv marked this pull request as ready for review December 1, 2023 19:49
@dagardner-nv dagardner-nv marked this pull request as draft December 11, 2023 18:23
@dagardner-nv dagardner-nv changed the title Make logits a constructor argument to TritonInferenceStage Decouple TritonInferenceStage from pipeline mode Dec 11, 2023
@dagardner-nv dagardner-nv marked this pull request as ready for review December 11, 2023 19:53
Copy link
Contributor

@mdemoret-nv mdemoret-nv left a comment

Choose a reason for hiding this comment

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

Make the following changes:

  • Remove AE support from the triton stage all together
  • Remove all implementations of the worker class
  • Make all options of the worker class instance values (need_logits, and inout_mapping)
  • Set defaults for inout_mapping similar to default for need_logits (i.e. by mode if not supplied)

@dagardner-nv dagardner-nv added the skip-ci Optionally Skip CI for this PR label Dec 20, 2023
@dagardner-nv dagardner-nv removed the skip-ci Optionally Skip CI for this PR label Dec 26, 2023
Copy link
Contributor

@mdemoret-nv mdemoret-nv left a comment

Choose a reason for hiding this comment

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

Lets get these changes merged but make an issue before you do to try and remove the inference worker class entirely.

@dagardner-nv
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 2e3f4e4 into nv-morpheus:branch-24.03 Jan 8, 2024
12 checks passed
@dagardner-nv dagardner-nv deleted the triton-logits-1378 branch January 8, 2024 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change improvement Improvement to existing functionality
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[FEA]: Make logits a constructor argument to TritonInferenceStage
3 participants