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

Add Dynamic Recurrent Basket Model (DREAM) #590

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

lthoang
Copy link
Member

@lthoang lthoang commented Jan 19, 2024

Description

Related Issues

#579

Checklist:

  • I have added tests.
  • I have updated the documentation accordingly.
  • I have updated README.md (if you are adding a new model).
  • I have updated examples/README.md (if you are adding a new example).
  • I have updated datasets/README.md (if you are adding a new dataset).

@lthoang lthoang requested a review from tqtg January 19, 2024 18:30
@lthoang lthoang added the models New models, changes to models label Jan 20, 2024
from tqdm.auto import trange


class Wloss(nn.modules.loss._Loss):
Copy link
Member

Choose a reason for hiding this comment

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

What is W loss? Can it be more informative or is it an acronym by the authors?

Copy link
Member Author

Choose a reason for hiding this comment

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

@tqtg I am not quite sure about the acronym for Wloss. The current implementation is based on this implemenation https://github.com/liming-7/A-Next-Basket-Recommendation-Reality-Check/tree/main/methods/dream

The authors of the above implementation also refer the running source code to this repo https://github.com/yihong-chen/DREAM

However, these two implementations are quite different in their naming convention. I haven't read through them thoroughly. Let's take some time to check the validity of both implementations, whether they reflect the paper idea.

Copy link
Member

Choose a reason for hiding this comment

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

Glancing at the paper, I only see BPR loss is used without additional information. I think it's better to rely on the original implementation which contains 2 variants (BPR and reordered BPR).

README.md Outdated Show resolved Hide resolved
@lthoang lthoang changed the title Add Dynamic REcurrent bAsket Model (DREAM) Add Dynamic Recurrent Basket Model (DREAM) Jan 21, 2024
self.loss_fct = nn.BCELoss()
self.p_loss_fct = Wloss(self.loss_uplift, 1)
self.n_loss_fct = Wloss(1, self.loss_uplift)
self.meta_loss_fct = nn.MSELoss()
Copy link
Member

Choose a reason for hiding this comment

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

I think meta_loss_fct is not used anywhere during training.

@tqtg
Copy link
Member

tqtg commented Feb 20, 2024

@lthoang are we actively working on this?

@lthoang
Copy link
Member Author

lthoang commented Feb 20, 2024

@lthoang are we actively working on this?

Yes. I will be back to this model next week.

@lthoang lthoang marked this pull request as draft March 1, 2024 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
models New models, changes to models
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants