Skip to content

RSDK-4197: Add support for Flat Tensors in Python SDK #425

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

Merged
merged 9 commits into from
Sep 11, 2023

Conversation

hexbabe
Copy link
Member

@hexbabe hexbabe commented Sep 7, 2023

Use numpy arrays as the backing data struct instead of dicts for mlmodel

Test plan notes:

  • EDGE FlatTensors without any tensors

    • Expected behavior: ummm I think just a empty dict
  • FlatTensors with one tensor (say.. double type)

    • Expected behavior: one entry dict with same name and same structured values as google.protobuf.internal.containers.RepeatedScalarFieldContainer
      • Check dtype is float64
  • FlatTensors with two tensors (double type, float type)

    • Expected behavior: two entry dict with same names and same structured values as google.protobuf.internal.containers.RepeatedScalarFieldContainer
      • Check that dtype matches double and float respectively
      • Check that the shape of the float one is 2, 2
  • Check all flat tensor data types in bundles of appropriate types

hexbabe added 3 commits September 6, 2023 16:43
… np conversion bidirectionally (first draft); changed client and service to use new updated params and new utils
@hexbabe hexbabe marked this pull request as ready for review September 8, 2023 14:05
@hexbabe hexbabe requested a review from a team as a code owner September 8, 2023 14:05
@hexbabe hexbabe requested review from stuqdog and cheukt September 8, 2023 14:05

ndarrays: Dict[str, NDArray] = dict()
for name, flat_tensor in flat_tensors.tensors.items():
property_name = flat_tensor.WhichOneof("tensor") or flat_tensor.WhichOneof(b"tensor") # sus...
Copy link
Member

Choose a reason for hiding this comment

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

This seems fine to me? Agreed it's awkward but that's what the method calls for :/

Probably good to remove the comment here before we merge, also.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you have more info than I on why the method accepts a bytes and non-bytes str literal of the same str?

Copy link
Member

Choose a reason for hiding this comment

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

I do not! but it is what it is 🤷

hexbabe added 2 commits September 8, 2023 12:47
…sor_data_class dict to outside of double nested helper to optimize instantiation; fixed timeout typo
@hexbabe hexbabe requested a review from stuqdog September 8, 2023 16:57
Copy link
Member

@stuqdog stuqdog left a comment

Choose a reason for hiding this comment

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

lgtm

@hexbabe hexbabe merged commit b210257 into viamrobotics:main Sep 11, 2023
data = ndarray.flatten()
if tensor_data_class == FlatTensorDataInt8 or tensor_data_class == FlatTensorDataUInt8:
data = data.tobytes() # as per the proto, int8 and uint8 are stored as bytes
elif tensor_data_class == FlatTensorDataInt16 or tensor_data_class == FlatTensorDataUInt16:
Copy link
Member Author

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants