add news rec methods NAML NRMS NPA LSTUR #1080

yjw1029 · 2020-04-02T16:31:51Z

Description

Add news rec methods (NAML NRMS NPA LSTUR)

Related Issues

#1079

Checklist:

I have followed the contribution guidelines and code style for this project.
I have added tests covering my contributions.
I have updated the documentation accordingly.
This PR is being made to staging and not master.

review-notebook-app · 2020-04-02T16:31:57Z

Check out this pull request on

You'll be able to see Jupyter notebook diff and discuss changes. Powered by ReviewNB.

msftclas · 2020-04-02T16:32:04Z

All CLA requirements met.

yueguoguo

Thanks for the codes! In general it looks good to me. Just provided some minor comments.

Btw, the tests on Spark environment failed and I believe there is no Spark related code submitted in the PR, is that correct?

cc @miguelgfierro

README.md

reco_utils/recommender/deeprec/deeprec_utils.py

reco_utils/recommender/newsrec/IO/iterator.py

reco_utils/recommender/newsrec/models/base_model.py

miguelgfierro · 2020-04-06T10:27:31Z

Btw, the tests on Spark environment failed and I believe there is no Spark related code submitted in the PR, is that correct?

Right now the test machines are down because of the measurements with covid in all MS. We will need to run the tests ourselves. I will add more folks to comment in the review

notebooks/00_quick_start/lstur_synthetic.ipynb

notebooks/00_quick_start/naml_synthetic.ipynb

notebooks/00_quick_start/npa_synthetic.ipynb

miguelgfierro

I took a look, this is really impressive guys. My honest congratulations. It is absolutely awesome to be collaborating with you. Please send my regards to Xing.

reco_utils/recommender/deeprec/deeprec_utils.py

reco_utils/recommender/newsrec/models/layers.py

reco_utils/recommender/newsrec/models/lstur.py

reco_utils/recommender/newsrec/models/naml.py

gramhagen

Nice additions! I added some small suggestions / questions.

gramhagen · 2020-04-06T12:04:20Z

README.md

+| Neural News Recommendation with Long- and Short-term User Representations (LSTUR) | [Python CPU / Python GPU](notebooks/00_quick_start/lstur_synthetic.ipynb) | Content-Based Filtering | Deep learning news recommendation algorithm considering users' long- and short-term preference |
+| Neural News Recommendation with Attentive Multi-View Learning (NAML) | [Python CPU / Python GPU](notebooks/00_quick_start/naml_synthetic.ipynb) | Content-Based Filtering | Deep learning news recommendation algorithm with multi-view learning |
 | Neural Collaborative Filtering (NCF) | [Python CPU / Python GPU](notebooks/00_quick_start/ncf_movielens.ipynb) | Collaborative Filtering | Deep learning algorithm with enhanced performance for implicit feedback |
+| Neural News Recommendation with Personalized Attention (NPA) | [Python CPU / Python GPU](notebooks/00_quick_start/npa_synthetic.ipynb) | Content-Based Filtering | Deep learning news recommendation algorithm ultilizing personalized attention |
+| Neural News Recommendation with Multi-Head Self-Attention (NRMS) | [Python CPU / Python GPU](notebooks/00_quick_start/nrms_synthetic.ipynbb) | Content-Based Filtering | Deep learning news recommendation algorithm ultilizing multi-head self-attention |


The rest of the algorithms in this table are not domain specific. Is it important to have the news focus here? It may lead to people overlooking it as an option for other domains.

reco_utils/recommender/newsrec/IO/news_iterator.py

gramhagen · 2020-04-06T12:20:47Z

reco_utils/recommender/newsrec/IO/news_iterator.py

+        return [imp_indexes, user_indexes, click_news_index_batch, candidate_news_index_batch], labels
+
+
+class NewsTestIterator(BaseIterator):


is this the same as NewsTrainIterator with npratio=0? Do we need separate classes? Or can we extend that class to reduce redundant code?

Thanks for the suggestions. I will try to change the code and combine these two iterators.

reco_utils/recommender/newsrec/IO/news_iterator.py

reco_utils/recommender/newsrec/models/base_model.py

reco_utils/recommender/newsrec/newsrec_utils.py

tests/smoke/test_newsrec_model.py

notebooks/00_quick_start/naml_synthetic.ipynb

Co-Authored-By: Scott Graham <5720537+gramhagen@users.noreply.github.com>

notebooks/00_quick_start/README.md

notebooks/00_quick_start/lstur_synthetic.ipynb

This reverts commit b53c4a3.

This reverts commit 082749a.

This reverts commit 7ac68fa.

README.md

tests/unit/test_newsrec_model.py

tests/smoke/test_newsrec_model.py

tests/unit/test_newsrec_utils.py

miguelgfierro · 2020-04-16T10:43:52Z

hey @yjw1029 @yueguoguo @gramhagen @anargyri, just FYI, I run all the tests in Prometheus, new and old ones and everything passes

…w1029/recommenders into v-jinyi/add-news-reco-methods

yjw1029 · 2020-04-23T06:22:00Z

hi @miguelgfierro. Thanks for your comment. I change the problems you mentioned above😁. Are there any issues in the code?

miguelgfierro · 2020-04-23T11:28:35Z

@yjw1029 from my part this is ready to go.

@gramhagen is there anything else that you would like to review?

miguelgfierro · 2020-04-24T12:19:05Z

after speaking with @gramhagen, I'll merge this.

@yjw1029 amazing work

add news rec nethods

43cd644

yjw1029 requested review from gramhagen, miguelgfierro and yueguoguo as code owners April 2, 2020 16:31

change group ndcg name to ndcg

9a5d6ef

yueguoguo approved these changes Apr 4, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

reco_utils/recommender/deeprec/deeprec_utils.py Show resolved Hide resolved

reco_utils/recommender/newsrec/IO/iterator.py Outdated Show resolved Hide resolved

reco_utils/recommender/newsrec/models/base_model.py Show resolved Hide resolved

yjw1029 added 2 commits April 5, 2020 12:55

reuse BaseIterator(

eaddd8b

remove powerful statement in README

a498967

miguelgfierro mentioned this pull request Apr 6, 2020

V jinyi/add news reco methods #1081

Closed

4 tasks

Merge branch 'staging' into v-jinyi/add-news-reco-methods

c9a5191

miguelgfierro requested review from Leavingseason, anargyri and loomlike April 6, 2020 10:30

miguelgfierro reviewed Apr 6, 2020

View reviewed changes

notebooks/00_quick_start/lstur_synthetic.ipynb Show resolved Hide resolved

notebooks/00_quick_start/naml_synthetic.ipynb Show resolved Hide resolved

notebooks/00_quick_start/naml_synthetic.ipynb Show resolved Hide resolved

notebooks/00_quick_start/npa_synthetic.ipynb Show resolved Hide resolved

miguelgfierro approved these changes Apr 6, 2020

View reviewed changes

gramhagen requested changes Apr 6, 2020

View reviewed changes

anargyri reviewed Apr 7, 2020

View reviewed changes

notebooks/00_quick_start/naml_synthetic.ipynb Show resolved Hide resolved

anargyri approved these changes Apr 8, 2020

View reviewed changes

yjw1029 and others added 5 commits April 8, 2020 21:37

Update reco_utils/recommender/newsrec/IO/news_iterator.py

bad129e

Co-Authored-By: Scott Graham <5720537+gramhagen@users.noreply.github.com>

using get in reco_utils/recommender/newsrec/newsrec_utils.py

cef1bbf

Co-Authored-By: Scott Graham <5720537+gramhagen@users.noreply.github.com>

using config.update in newsrec_utils.py

fcb0b7e

Co-Authored-By: Scott Graham <5720537+gramhagen@users.noreply.github.com>

merge train adn test iterator

e5a7ad3

change all if else in create_hparams into get

fb6ebf5

miguelgfierro reviewed Apr 8, 2020

View reviewed changes

notebooks/00_quick_start/README.md Outdated Show resolved Hide resolved

anargyri reviewed Apr 8, 2020

View reviewed changes

notebooks/00_quick_start/lstur_synthetic.ipynb Show resolved Hide resolved

yjw1029 added 2 commits April 8, 2020 16:06

activate black | merge iterators | use tmp | iter dict

b6a98cf

change typo in README and lstu_synthetic.ipynb

10a7ec1

yjw1029 added 7 commits April 8, 2020 16:40

change unit and smoke test

b53c4a3

merge news rec method

7ac68fa

Revert "change unit and smoke test"

082749a

This reverts commit b53c4a3.

Revert "Revert "change unit and smoke test""

f02d437

This reverts commit 082749a.

Revert "merge news rec method"

dbb87d1

This reverts commit 7ac68fa.

add integration test for NAML NRMS NPA LSTUR

5a3dfdc

change desc of algos in README

830a6cc

miguelgfierro reviewed Apr 15, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

miguelgfierro reviewed Apr 15, 2020

View reviewed changes

tests/unit/test_newsrec_model.py Outdated Show resolved Hide resolved

tests/smoke/test_newsrec_model.py Outdated Show resolved Hide resolved

Merge branch 'staging' into v-jinyi/add-news-reco-methods

71d01b6

miguelgfierro reviewed Apr 15, 2020

View reviewed changes

tests/unit/test_newsrec_utils.py Outdated Show resolved Hide resolved

tests/unit/test_newsrec_utils.py Show resolved Hide resolved

miguelgfierro and others added 6 commits April 22, 2020 07:05

Merge branch 'staging' into v-jinyi/add-news-reco-methods

faca2a3

add <sup>*</sup> for NAML NRMS LSTUR NPA in README.md

fb48a98

remove all data_path=tmp and remove algo related mark

50cd5bc

remove mark.newsrec

f4653c2

speed up test_naml_iterator

c7be021

Merge branch 'v-jinyi/add-news-reco-methods' of https://github.com/yj…

e78a40d

…w1029/recommenders into v-jinyi/add-news-reco-methods

Merge branch 'staging' into v-jinyi/add-news-reco-methods

2528204

miguelgfierro added 2 commits April 24, 2020 09:13

Merge branch 'staging' into v-jinyi/add-news-reco-methods

a06f965

Merge branch 'staging' into v-jinyi/add-news-reco-methods

1cc5a86

miguelgfierro merged commit 10d90d8 into recommenders-team:staging Apr 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add news rec methods NAML NRMS NPA LSTUR #1080

add news rec methods NAML NRMS NPA LSTUR #1080

yjw1029 commented Apr 2, 2020 •

edited

review-notebook-app bot commented Apr 2, 2020

msftclas commented Apr 2, 2020 •

edited

yueguoguo left a comment

miguelgfierro commented Apr 6, 2020

miguelgfierro left a comment

gramhagen left a comment

gramhagen Apr 6, 2020

gramhagen Apr 6, 2020

yjw1029 Apr 7, 2020

miguelgfierro commented Apr 16, 2020

yjw1029 commented Apr 23, 2020

miguelgfierro commented Apr 23, 2020

miguelgfierro commented Apr 24, 2020

		return [imp_indexes, user_indexes, click_news_index_batch, candidate_news_index_batch], labels


		class NewsTestIterator(BaseIterator):

add news rec methods NAML NRMS NPA LSTUR #1080

add news rec methods NAML NRMS NPA LSTUR #1080

Conversation

yjw1029 commented Apr 2, 2020 • edited

Description

Related Issues

Checklist:

review-notebook-app bot commented Apr 2, 2020

msftclas commented Apr 2, 2020 • edited

yueguoguo left a comment

Choose a reason for hiding this comment

miguelgfierro commented Apr 6, 2020

miguelgfierro left a comment

Choose a reason for hiding this comment

gramhagen left a comment

Choose a reason for hiding this comment

gramhagen Apr 6, 2020

Choose a reason for hiding this comment

gramhagen Apr 6, 2020

Choose a reason for hiding this comment

yjw1029 Apr 7, 2020

Choose a reason for hiding this comment

miguelgfierro commented Apr 16, 2020

yjw1029 commented Apr 23, 2020

miguelgfierro commented Apr 23, 2020

miguelgfierro commented Apr 24, 2020

yjw1029 commented Apr 2, 2020 •

edited

msftclas commented Apr 2, 2020 •

edited