Skip to content

[REVIEW] Add nvtext::porter_stemmer_measure API - #5767

Merged
davidwendt merged 25 commits into
NVIDIA:branch-0.15from
davidwendt:fea-nvtext-porter-stemmer
Aug 5, 2020
Merged

davidwendt merged 25 commits into
NVIDIA:branch-0.15from
davidwendt:fea-nvtext-porter-stemmer

Conversation

@davidwendt

Copy link
Copy Markdown
Contributor

References #5243

This PR adds nvtext APIs to unblock NVIDIA/cuml#2476
It includes two new APIs ported from the old NVText implementation as referenced in #5243
The algorithm is based on https://tartarus.org/martin/PorterStemmer/def.txt

std::unique_ptr<cudf::column> porter_stemmer_measure(
  cudf::strings_column_view const& strings,
  rmm::mr::device_memory_resource* mr);
std::unique_ptr<cudf::column> is_letter(
  cudf::strings_column_view const& strings,
  letter_type ltype,
  cudf::size_type character_index,  ///---- also a version that takes a column of indices here instead
  rmm::mr::device_memory_resource* mr)

The main difference is since this algorithm only works on English, the aeiou and y characters are hardcoded inside the implementation. The previous NVText version allowed these to be parameters.

This PR will include adding the Python interface as well.

@davidwendt davidwendt self-assigned this Jul 24, 2020
@davidwendt davidwendt added 2 - In Progress Currently a work in progress libcudf Affects libcudf (C++/CUDA) code. strings strings issues (C++ and Python) labels Jul 24, 2020
@GPUtester

Copy link
Copy Markdown
Collaborator

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

@davidwendt

Copy link
Copy Markdown
Contributor Author

rerun tests

@codecov

codecov Bot commented Jul 27, 2020

Copy link
Copy Markdown

Codecov Report

Merging #5767 into branch-0.15 will increase coverage by 0.33%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.15    #5767      +/-   ##
===============================================
+ Coverage        84.08%   84.41%   +0.33%     
===============================================
  Files               80       80              
  Lines            13062    13395     +333     
===============================================
+ Hits             10983    11308     +325     
- Misses            2079     2087       +8     
Impacted Files Coverage Δ
python/cudf/cudf/core/column/string.py 86.82% <100.00%> (+0.41%) ⬆️
python/cudf/cudf/io/feather.py 100.00% <0.00%> (ø)
python/cudf/cudf/comm/serialize.py 0.00% <0.00%> (ø)
python/custreamz/custreamz/_version.py 0.00% <0.00%> (ø)
python/dask_cudf/dask_cudf/_version.py 0.00% <0.00%> (ø)
python/dask_cudf/dask_cudf/io/tests/test_orc.py 100.00% <0.00%> (ø)
python/dask_cudf/dask_cudf/io/tests/test_json.py 100.00% <0.00%> (ø)
...ython/dask_cudf/dask_cudf/io/tests/test_parquet.py 100.00% <0.00%> (ø)
python/cudf/cudf/utils/applyutils.py 98.75% <0.00%> (+0.02%) ⬆️
python/cudf/cudf/core/join/join.py 92.41% <0.00%> (+0.03%) ⬆️
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7245b1...b495781. Read the comment docs.

@davidwendt davidwendt changed the title [WIP] Add nvtext::porter_stemmer_measure API [REVIEW] Add nvtext::porter_stemmer_measure API Jul 27, 2020
@davidwendt davidwendt added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Jul 27, 2020
@davidwendt
davidwendt marked this pull request as ready for review July 27, 2020 23:08
@davidwendt
davidwendt requested review from a team as code owners July 27, 2020 23:08
@davidwendt

Copy link
Copy Markdown
Contributor Author

rerun tests

Comment thread python/cudf/cudf/_lib/cpp/nvtext/stemmer.pxd
Comment thread python/cudf/cudf/core/column/string.py Outdated
Comment thread python/cudf/cudf/core/column/string.py Outdated
Comment thread python/cudf/cudf/_lib/nvtext/stemmer.pyx Outdated
Comment thread python/cudf/cudf/_lib/nvtext/stemmer.pyx Outdated
@kkraus14 kkraus14 added the 4 - Needs Review Waiting for reviewer to review or respond label Jul 28, 2020
Comment thread cpp/src/text/stemmer.cu
Comment thread cpp/src/text/stemmer.cu
Comment thread cpp/src/text/stemmer.cu

@ramakrishnap-nv ramakrishnap-nv left a comment

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.

LGTM

Comment thread cpp/src/text/stemmer.cu
@ramakrishnap-nv

Copy link
Copy Markdown
Contributor

Don't merge this as one cpp review is still pending.

@codereport codereport left a comment

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 code looks great! And it is such a cool algorithm. Only 1 small change.

Comment thread cpp/src/text/stemmer.cu
Comment thread cpp/src/text/stemmer.cu Outdated
Comment thread cpp/src/text/stemmer.cu Outdated
@codereport codereport added 0 - Waiting on Author Waiting for author to respond to review and removed 4 - Needs Review Waiting for reviewer to review or respond labels Aug 5, 2020
@davidwendt davidwendt removed the 0 - Waiting on Author Waiting for author to respond to review label Aug 5, 2020
@davidwendt
davidwendt requested a review from codereport August 5, 2020 12:35

@codereport codereport left a comment

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.

Looks great 👍

@davidwendt
davidwendt merged commit 23b3af6 into NVIDIA:branch-0.15 Aug 5, 2020
@davidwendt
davidwendt deleted the fea-nvtext-porter-stemmer branch August 5, 2020 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. strings strings issues (C++ and Python)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants