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

Adopt matx v0.3.0 #667

Merged

Conversation

dagardner-nv
Copy link
Contributor

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

  • Remove usage of tensorShape_t which was deprecated, and later removed.
  • Replace usage of tensor constructor in favor of the recommended make_tensor helper method.
  • Adds more C++ unittests
  • RMMTensor marked as a public symbol so the C++ tests can use it
  • Add cuda-nvtx package to CI driver build, needed for matx-0.3.0

Includes changes from PR #688
fixes #317

@dagardner-nv dagardner-nv added non-breaking Non-breaking change improvement Improvement to existing functionality 2 - In Progress labels Feb 3, 2023
@dagardner-nv dagardner-nv requested a review from a team as a code owner February 3, 2023 19:31
… mapping back

to the same source row in the dataframe.

ex:
model max batch = 4
seq_ids = [0, 0, 0, 1, 1, 2, 3, 3]

Would cause the inference inputs for dataframe row 1
to occurr in two non-adjacent batches.

This is a non-ideal fix as this doesn't handle the case where the inference
input is so large that it spans more rows than the model's max batch size.
… mapping back

to the same source row in the dataframe.

ex:
model max batch = 4
seq_ids = [0, 0, 0, 1, 1, 2, 3, 3]

Would cause the inference inputs for dataframe row 1
to occurr in two non-adjacent batches.

This is a non-ideal fix as this doesn't handle the case where the inference
input is so large that it spans more rows than the model's max batch size.
@dagardner-nv dagardner-nv requested a review from a team as a code owner February 9, 2023 19:56
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.

Just a couple of questions. But looks good otherwise.

ci/runner/Dockerfile Show resolved Hide resolved
morpheus/_lib/src/utilities/matx_util.cu Show resolved Hide resolved
@mdemoret-nv
Copy link
Contributor

/merge

@rapids-bot rapids-bot bot merged commit 1ab97d3 into nv-morpheus:branch-23.03 Feb 22, 2023
jjacobelli pushed a commit to jjacobelli/Morpheus that referenced this pull request Mar 7, 2023
* Remove usage of `tensorShape_t` which was deprecated, and later removed.
* Replace usage of tensor constructor in favor of the recommended `make_tensor` helper method.
* Adds more C++ unittests
* RMMTensor marked as a public symbol so the C++ tests can use it
* Add `cuda-nvtx` package to CI driver build, needed for matx-0.3.0

Includes changes from PR nv-morpheus#688
fixes nv-morpheus#317

Authors:
  - David Gardner (https://github.com/dagardner-nv)

Approvers:
  - Michael Demoret (https://github.com/mdemoret-nv)

URL: nv-morpheus#667
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement to existing functionality non-breaking Non-breaking change
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[FEA] Adopt newer version of Matx
2 participants