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.

Sign up for GitHub

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

Jump to bottom

Add struct column support to cudf_helpers #1538

Merged
merged 38 commits into from
Jun 18, 2024
Merged

Add struct column support to cudf_helpers #1538

merged 38 commits into from
Jun 18, 2024

Conversation

efajardo-nv
Copy link
Contributor

@efajardo-nv efajardo-nv commented Feb 28, 2024 •
edited
Loading

Description

  • Update make_table_from_table_info_data in cudf_helpers to build a table_metadata object used to add support for struct columns.
  • Update DatasetManager.df_equal to support list columns. Now uses pandas equals if val_to_check is a DataFrame of Series.
  • Add new jsonlines file to use for building test DataFrame that includes struct and list columns.
  • Update MessageMeta and ControlMessage tests to use new test data.

Closes #1527

By Submitting this PR I confirm:

  • I am familiar with the Contributing Guidelines.
  • When the PR is ready for review, new or existing tests cover these changes.
  • When the PR is ready for review, the documentation is up to date with these changes.

@efajardo-nv efajardo-nv added bug Something isn't working non-breaking Non-breaking change labels Feb 28, 2024
mdemoret-nv
mdemoret-nv requested changes Feb 28, 2024
View reviewed changes
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.

Good start but lets find all uses of the get_data_frame() function and convert them to use copy_dataframe() and get_mutable_dataframe()

morpheus/_lib/src/messages/meta.cpp Outdated Show resolved Hide resolved
morpheus/_lib/src/messages/meta.cpp Outdated Show resolved Hide resolved
@efajardo-nv efajardo-nv self-assigned this Feb 29, 2024
@efajardo-nv efajardo-nv changed the title [DRAFT] Update morpheus._lib.messages.MessageMeta.get_data_frame() to use cuDF deep copy [DRAFT] Add struct column support to cudf_helpers Mar 6, 2024
@efajardo-nv efajardo-nv changed the title [DRAFT] Add struct column support to cudf_helpers Add struct column support to cudf_helpers Mar 6, 2024
@efajardo-nv efajardo-nv marked this pull request as ready for review March 6, 2024 19:39
@efajardo-nv efajardo-nv requested a review from a team as a code owner March 6, 2024 19:39
@dagardner-nv dagardner-nv mentioned this pull request Mar 6, 2024
Closed
@efajardo-nv efajardo-nv changed the title Add struct column support to cudf_helpers [DRAFT] Add struct column support to cudf_helpers Mar 8, 2024
@efajardo-nv efajardo-nv added the DO NOT MERGE PR should not be merged; see PR for details label Mar 8, 2024
@efajardo-nv efajardo-nv changed the title [DRAFT] Add struct column support to cudf_helpers Add struct column support to cudf_helpers Mar 8, 2024
@efajardo-nv efajardo-nv removed the DO NOT MERGE PR should not be merged; see PR for details label Mar 8, 2024
mdemoret-nv
mdemoret-nv reviewed Mar 13, 2024
View reviewed changes
ci/scripts/common.sh Outdated Show resolved Hide resolved
tests/messages/test_message_meta.py Show resolved Hide resolved
@efajardo-nv efajardo-nv mentioned this pull request Mar 19, 2024
Closed
2 tasks
@mdemoret-nv mdemoret-nv changed the base branch from branch-24.03 to branch-24.06 March 28, 2024 18:20
@copy-pr-bot copy-pr-bot
Copy link

copy-pr-bot bot commented May 23, 2024

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@AnuradhaKaruppiah AnuradhaKaruppiah force-pushed the messagemeta-get-dataframe-update branch 2 times, most recently from 6ef268b to f7f1a2c Compare May 23, 2024 19:06
@AnuradhaKaruppiah
unit-tests: Add a test to update and validate a struct df
9f8e048 
df is updated, passed to cpp and copied back to verify update

Signed-off-by: Anuradha Karuppiah <anuradhak@nvidia.com>
@AnuradhaKaruppiah AnuradhaKaruppiah force-pushed the messagemeta-get-dataframe-update branch from f7f1a2c to 9f8e048 Compare May 23, 2024 19:15
@mdemoret-nv
Copy link
Contributor

/ok to test

@mdemoret-nv
Copy link
Contributor

/ok to test

mdemoret-nv
mdemoret-nv approved these changes Jun 18, 2024
View reviewed changes
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.

All tests now passing. Approved

@mdemoret-nv
Copy link
Contributor

/merge

@rapids-bot rapids-bot bot merged commit 46f842d into nv-morpheus:branch-24.06 Jun 18, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mdemoret-nv mdemoret-nv mdemoret-nv approved these changes

Assignees

@efajardo-nv efajardo-nv

Labels
bug Something isn't working non-breaking Non-breaking change
Projects
Morpheus Boards
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG]: morpheus._lib.messages.MessageMeta not preserving key names in cudf.core.dtypes.StructDtype columns
3 participants
@efajardo-nv @mdemoret-nv @AnuradhaKaruppiah