Skip to content

[REVIEW]Porter Stemmer - #2476

Merged
JohnZed merged 17 commits into
NVIDIA:branch-0.16from
VibhuJawa:fea_porter_stemmer
Sep 24, 2020
Merged

[REVIEW]Porter Stemmer#2476
JohnZed merged 17 commits into
NVIDIA:branch-0.16from
VibhuJawa:fea_porter_stemmer

Conversation

@VibhuJawa

@VibhuJawa VibhuJawa commented Jun 24, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

This pr adds a porter stemmer implementation to cuml.

With this implementation we are currently at 39x to nltk-porterstemmer .

See gist for accuracy tests and benchmarking:

  • Benchmark (39x Faster)
  • Add license related headers
  • Add tests
  • Edit docstring of the Porter Stemmer API with example
  • Add to api-docs

@GPUtester

GPUtester commented Jun 24, 2020

Copy link
Copy Markdown
Contributor

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

@VibhuJawa

VibhuJawa commented Jun 24, 2020

Copy link
Copy Markdown
Contributor Author

CC: @randerzander / @cjnolet

@cjnolet cjnolet self-assigned this Jun 24, 2020
@cjnolet cjnolet added 2 - In Progress Currenty a work in progress Cython / Python Cython or Python issue New Algorithm For tracking new algorithms that will be added to our existing collection labels Jun 24, 2020
This was referenced Jul 24, 2020
Merged
Merged
@VibhuJawa
VibhuJawa changed the base branch from branch-0.15 to branch-0.16 August 17, 2020 16:31
@VibhuJawa
VibhuJawa changed the base branch from branch-0.16 to branch-0.5 September 2, 2020 23:28
@VibhuJawa
VibhuJawa changed the base branch from branch-0.5 to branch-0.16 September 2, 2020 23:28
@VibhuJawa VibhuJawa changed the title [WIP]Porter Stemmer[skip-ci] [WIP]Porter Stemmer Sep 2, 2020
@VibhuJawa
VibhuJawa marked this pull request as ready for review September 2, 2020 23:28
@VibhuJawa
VibhuJawa requested a review from a team as a code owner September 2, 2020 23:28
@VibhuJawa

VibhuJawa commented Sep 2, 2020

Copy link
Copy Markdown
Contributor Author

rerun tests

VibhuJawa
VibhuJawa commented Sep 2, 2020
View reviewed changes
Comment thread python/cuml/preprocessing/text/stem/porter_stemmer.py

# Implimentation based on nltk//stem/porter.html
# https://www.nltk.org/_modules/nltk/stem/porter.html
class PorterStemmer:

@VibhuJawa VibhuJawa Sep 2, 2020

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.

This is the main logic of the program.

@VibhuJawa

VibhuJawa commented Sep 2, 2020

Copy link
Copy Markdown
Contributor Author

rerun tests

@VibhuJawa VibhuJawa changed the title [WIP]Porter Stemmer [REVIEW]Porter Stemmer Sep 3, 2020
@VibhuJawa

VibhuJawa commented Sep 3, 2020

Copy link
Copy Markdown
Contributor Author

This should be ready for a first review.

@VibhuJawa

VibhuJawa commented Sep 8, 2020

Copy link
Copy Markdown
Contributor Author

@cjnolet , Can I get a review of this PR when you have time, please.

cjnolet
cjnolet requested changes Sep 10, 2020
View reviewed changes

@cjnolet cjnolet left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This implementation looks great. It's a lot to go through so I've done a first pass and offered some feedback. All minor things so far.

Comment thread python/cuml/preprocessing/text/stem/porter_stemmer.py Outdated
Comment thread python/cuml/preprocessing/text/stem/porter_stemmer.py Outdated
Comment thread python/cuml/preprocessing/text/stem/porter_stemmer_utils/suffix_utils.py Outdated
Comment thread python/cuml/test/stemmer_tests/test_steps.py
Comment thread python/cuml/test/stemmer_tests/test_steps.py
@VibhuJawa @cjnolet
Update python/cuml/preprocessing/text/stem/porter_stemmer.py
3a0505c 
Co-authored-by: Corey J. Nolet <cjnolet@users.noreply.github.com>
@VibhuJawa VibhuJawa mentioned this pull request Sep 15, 2020
Merged
@VibhuJawa VibhuJawa changed the title [REVIEW]Porter Stemmer [WIP]Porter Stemmer Sep 15, 2020
@VibhuJawa

VibhuJawa commented Sep 15, 2020

Copy link
Copy Markdown
Contributor Author

rerun tests

@VibhuJawa VibhuJawa changed the title [WIP]Porter Stemmer [REVIEW]Porter Stemmer Sep 15, 2020
@cjnolet

cjnolet commented Sep 18, 2020 •
edited
Loading

Copy link
Copy Markdown
Collaborator

@VibhuJawa , is this ready for re-review?

@VibhuJawa

VibhuJawa commented Sep 18, 2020

Copy link
Copy Markdown
Contributor Author

@VibhuJawa , is this ready for re-review?

Yup It is.

@VibhuJawa

VibhuJawa commented Sep 18, 2020

Copy link
Copy Markdown
Contributor Author

rerun tests

1 similar comment
@VibhuJawa

VibhuJawa commented Sep 21, 2020

Copy link
Copy Markdown
Contributor Author

rerun tests

cjnolet
cjnolet approved these changes Sep 22, 2020
View reviewed changes

@cjnolet cjnolet left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@VibhuJawa

VibhuJawa commented Sep 23, 2020

Copy link
Copy Markdown
Contributor Author

rerun tests

@JohnZed
JohnZed merged commit a8fe745 into NVIDIA:branch-0.16 Sep 24, 2020
@JohnZed

JohnZed commented Sep 24, 2020

Copy link
Copy Markdown
Contributor

Thanks, @VibhuJawa ! Great change.

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

Reviewers

@cjnolet cjnolet cjnolet approved these changes

Assignees

@cjnolet cjnolet

Labels

2 - In Progress Currenty a work in progress Cython / Python Cython or Python issue New Algorithm For tracking new algorithms that will be added to our existing collection

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@VibhuJawa @GPUtester @cjnolet @JohnZed