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

Deprecate ResponseMemoryProbs & MultiResponseProbsMessage #711

Merged

Conversation

dagardner-nv
Copy link
Contributor

@dagardner-nv dagardner-nv commented Feb 15, 2023

  • Log a deprecation warning from the constructor for ResponseMemoryProbs & MultiResponseMemoryProbs
  • Replace usage of ResponseMemoryProbs with ResponseMemory
  • Replace usage of MultiResponseProbsMessage with MultiResponseMessage
  • AddClassificationsStage and AddScoresStage now accept an optional output_name="probs" constructor argument.
  • breaking Any custom stages which only return MultiResponseMemoryProbs from the accepted_types method will fail during pipeline build.
  • Includes changes in PR Fix Python bindings for TensorMemory #655

fixes #697

@dagardner-nv dagardner-nv added 3 - Ready for Review and removed Merge After Dependencies PR is completed and reviewed but depends on another PR; do not merge out of order 2 - In Progress labels Mar 7, 2023
@mdemoret-nv
Copy link
Contributor

Below is the final list of changes after all updates.

Breaking Changes

  • All memory classes, TensorMemory, ResponseMemory and InferenceMemory now require keyword only args (Necessary to ensure consistency between the classes)
  • ResponseMemory and InferenceMemory have been deprecated in favor of just using TensorMemory
    • This doesnt remove any functionality besides checks on required tensor names
  • AddScoresStage and AddClassificationStage require keyword only argument and the order has changed
    • This is due to the fact that they now inherit from a base class and should share constructor argument ordering

Additional Changes

  • The MultiTensorMessage class has a new property id_tensor_name
    • This property stores the name of the tensor to use for message ID lookup. Before this was hard coded to seq_ids and was inflexible
    • A new method, get_id_tensor() has been added to streamline calling get_tensor(self.id_tensor_name)
  • The MultiResponseMessage class has a new property probs_tensor_name
    • This property stores the name of the tensor that contains inference output probabilities. Before this was hard coded to probs and was inflexible
    • A new method, get_probs_tensor() has been added to streamline calling get_tensor(self.probs_tensor_name)

@mdemoret-nv mdemoret-nv requested a review from a team as a code owner March 21, 2023 18:53
@mdemoret-nv
Copy link
Contributor

/merge

@rapids-bot rapids-bot bot merged commit 940cd8c into nv-morpheus:branch-23.03 Mar 21, 2023
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
2 participants