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

Use size_t for element counts & byte sizes #1007

Merged

Conversation

dagardner-nv
Copy link
Contributor

@dagardner-nv dagardner-nv commented Jun 28, 2023

Description

  • Continue to use TensorIndex (int32_t) as an index in tensors & dataframes.
  • Use std::size_t for byte & element counts (for multi-dimensional arrays/tensors).
  • Use TensorIndex for element counts for 1d tensors/arrays and things like num_rows.

Although we can only have at most 2^31 elements in any dimension, we can have 2d tensors where the total elements exceeds 2^31 as well as a byte count that exceeds 2^31.

fixes #1004

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@dagardner-nv dagardner-nv requested a review from a team as a code owner June 28, 2023 15:34
@dagardner-nv dagardner-nv added bug Something isn't working breaking Breaking change 3 - Ready for Review labels Jun 28, 2023
@dagardner-nv dagardner-nv self-assigned this Jun 28, 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.

Looks good. But similar to how we have TensorIndex, can we have a TensorSize which we use in place of all the size_t? Then it will be easier to update in the future if we need.

morpheus/_lib/src/utilities/cupy_util.cpp Outdated Show resolved Hide resolved
morpheus/_lib/include/morpheus/utilities/tensor_util.hpp Outdated Show resolved Hide resolved
@mdemoret-nv
Copy link
Contributor

/merge

@rapids-bot rapids-bot bot merged commit a344b0d into nv-morpheus:branch-23.07 Jun 28, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[BUG]: Error in custom pipeline when pipeline_batch_size>=131072
2 participants