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

[FEA]: Improve developer experience with MessageMeta and MultiMessage classes #687

Closed
2 tasks done
mdemoret-nv opened this issue Feb 9, 2023 · 0 comments · Fixed by #691
Closed
2 tasks done

[FEA]: Improve developer experience with MessageMeta and MultiMessage classes #687

mdemoret-nv opened this issue Feb 9, 2023 · 0 comments · Fixed by #691
Labels
feature request New feature or request

Comments

@mdemoret-nv
Copy link
Contributor

Is this a new feature, an improvement, or a change to existing functionality?

Improvement

How would you describe the priority of this feature request

High

Please provide a clear description of problem this feature solves

How to correctly use the MessageMeta, MultiMessage and TensorMemory classes is a common error I see from developers when making custom pipelines. The main issue seems to stem around which values to use for offset, count, mess_offset and mess_count when changing or creating new MultiMessage classes.

Related to #674

Describe your ideal solution

We could improve the developer experience in a few ways:

  1. Better documentation in the developer guide
    1. We have few common scenarios that would be good to point out when creating a new MultiMessage object:
      1. If using an existing MessageMeta:
        1. In this scenario, you would want to copy the mess_offset and mess_count from the incoming message's MessageMeta.
      2. If creating a new MessageMeta at the same time:
        1. In this scenario, mess_offset = 0 and mess_count = MessageMeta.count
      3. If using an existing TensorMemory:
        1. In this scenario, you would want to copy the offset and count from the incoming message's TensorMemory.
      4. If creating a new TensorMemory at the same time:
        1. In this scenario, offset = 0 and count = TensorMemory.count
  2. Adding static functions with more overloads to create these classes for the developer in different scenarios
    1. One overload could be made for each of the above scenarios to remove the need for developers to manually set the values
  3. Adding assertions to the MessageMeta and MultiMessage classes to check for common mistakes
    1. For example, one common mistake is choosing offest and count options which are impossible. If the MessageMeta class only has 100 rows, then the following must be true MultiMessage.offset + MultiMessage.count <= MessageMeta.count. Otherwise you can index past the last item in the meta.

Describe any alternatives you have considered

No response

Additional context

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
  • I have searched the open feature requests and have found no duplicates for this feature request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant