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.

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

[BUG] SARSingleNode deprecated #989

Closed
konabuta opened this issue Nov 28, 2019 · 5 comments
Closed

[BUG] SARSingleNode deprecated #989

konabuta opened this issue Nov 28, 2019 · 5 comments
Labels
bug Something isn't working

Comments

@konabuta
Copy link

Description

sar_movielens_with_azureml.ipynb failed with error. It is because of SARSingleNode doesn't accept remove_seen.

In which platform does it happen?

Azure Machine Learning Notebook VM

How do we replicate the issue?

I followed this link Getting Started to install Python(reco) kernel. I just run the sar_movielens_with_azureml.ipynb.

You will get the error message like Error occurred: User program failed with TypeError: __init__() got an unexpected keyword argument 'remove_seen'

Expected behavior (i.e. solution)

Training script should pass successfully

Other Comments

I attached my notebook that works. I changed the following code.

  • SARSingleNode -> SAR
  • add remove_seen to model.recommend_k_items method

If there's no problem in my proposed chanage, I want to contribute to this project. Please provide label.
sar_movielens_with_azureml.ipynb.zip

@konabuta konabuta added the bug Something isn't working label Nov 28, 2019
@yueguoguo
Copy link
Collaborator

Hi @konabuta, the constructor of SARSingleNode does not take remove_seen as an input argument (see line here). It is instead used in the method of recommend_k_items (see line here).

In the notebook, it looks there is no remove_seen in the inputs of the constructor of the SARSingleNode (see here). Did you add that by yourself? Do note that the remove seen items operation only apply to the recommendation steps, and this is why it is not an input of the constructor itself.

Hope this clarifies.

@konabuta
Copy link
Author

I found the following code in this notebook. I don't add by myself.

model = SARSingleNode(
    remove_seen=True, similarity_type="jaccard", 
    time_decay_coefficient=30, time_now=None, timedecay_formula=True, **header
)

Could you check again ?

Thanks,
Keita

@miguelgfierro
Copy link
Collaborator

good catch @kanobuta, you are right, we need to change that. The correct code would be:

model = SAR(
    col_user="userID",
    col_item="itemID",
    col_rating="rating",
    col_timestamp="timestamp",
    similarity_type="jaccard", 
    time_decay_coefficient=30, 
    timedecay_formula=True
)

We would do a PR fixing this soon

@konabuta
Copy link
Author

konabuta commented Nov 29, 2019

@miguelgfierro Thank you for your confirmation ! I want to contribute to this repo directly next time :)

@miguelgfierro
Copy link
Collaborator

@konabuta that would be fantastic, feel free to take any issue or bug in the list https://github.com/microsoft/recommenders/issues. And thanks again for finding the bug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants