Skip to content

Improved Performance for Fixed List Columns#97

Closed
oliverholworthy wants to merge 7 commits intoNVIDIA-Merlin:mainfrom
oliverholworthy:fixed-list-improved-performance
Closed

Improved Performance for Fixed List Columns#97
oliverholworthy wants to merge 7 commits intoNVIDIA-Merlin:mainfrom
oliverholworthy:fixed-list-improved-performance

Conversation

@oliverholworthy
Copy link
Copy Markdown
Contributor

Goal: Load Fixed List Columns more quickly.

Columns with fixed and ragged list features currently take significanlty longer to load compared with scalar features. See #82

This is a first step toward making list features load more quickly.

Handling the special case of fixed list features where the lists are all of the same length, we can bypass some of the slower code that handles ragged lists and load these more quickly.

@oliverholworthy oliverholworthy added the enhancement New feature or request label Feb 8, 2023
@oliverholworthy oliverholworthy added this to the Merlin 23.02 milestone Feb 8, 2023
@oliverholworthy oliverholworthy self-assigned this Feb 8, 2023
@viswa-nvidia viswa-nvidia requested review from benfred and jperez999 and removed request for jperez999 February 14, 2023 17:46
@oliverholworthy oliverholworthy marked this pull request as ready for review February 20, 2023 09:55
except ImportError:
cp = np

try:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This could use cudf from merlin.core.compat but that's a tiny nit that need not block this PR 🤷🏻

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Once NVIDIA-Merlin/core#244 is merged we'll be able to. (There's still one thing to fix on that PR)

@oliverholworthy
Copy link
Copy Markdown
Contributor Author

Following the refactor of list handling #104 this PR has too many conflicts and demonstrates the difficulty we had with the code as it was before.

I've opened a new PR #112 that extracts and builds on the relevant parts from this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants