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 RecVAE model #568

Merged
merged 18 commits into from
Dec 26, 2023
Merged

Add RecVAE model #568

merged 18 commits into from
Dec 26, 2023

Conversation

flywithu
Copy link
Contributor

@flywithu flywithu commented Dec 11, 2023

Description

Add the option to use RecVAE model

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).

@flywithu flywithu changed the title Myrecvae2 Add RecVAE model Dec 11, 2023
@tqtg tqtg requested review from lthoang, saghiles and hieuddo December 12, 2023 02:01
@tqtg
Copy link
Member

tqtg commented Dec 13, 2023

Thanks for the contribution. This is great!

Glancing at the RecVAE model, I do notice that it shares a lot of similarities with some models that we have in Cornac (e.g., VAECF). Is it possible that you can restructure your code accordingly so we can better review and maintain? There are multiple utilities which can be reused instead of developing from scratch.

@flywithu
Copy link
Contributor Author

Thanks for the contribution. This is great!

Glancing at the RecVAE model, I do notice that it shares a lot of similarities with some models that we have in Cornac (e.g., VAECF). Is it possible that you can restructure your code accordingly so we can better review and maintain? There are multiple utilities which can be reused instead of developing from scratch.

"Thank you for your opinion. I will make the necessary revisions based on your request, referencing vaecf."

@flywithu
Copy link
Contributor Author

I've removed duplicate code such as batch code.

setup.py Outdated Show resolved Hide resolved
@flywithu flywithu requested a review from lthoang December 24, 2023 07:11
@lthoang lthoang requested a review from tqtg December 25, 2023 00:28
cornac/models/__init__.py Outdated Show resolved Hide resolved
cornac/models/recvae/recom_recvae.py Outdated Show resolved Hide resolved
examples/recvae_example.py Outdated Show resolved Hide resolved
cornac/models/recvae/recom_recvae.py Outdated Show resolved Hide resolved
cornac/models/recvae/recom_recvae.py Outdated Show resolved Hide resolved
@tqtg
Copy link
Member

tqtg commented Dec 25, 2023

hey @flywithu, this looks good to me overall. I just have a few comments above that can be easily addressed. Thanks again for this great contribution.

@flywithu
Copy link
Contributor Author

hi @tqtg
Thank you for your review and comments. I have also learned new.

@tqtg tqtg merged commit 477405a into PreferredAI:master Dec 26, 2023
13 checks passed
@tqtg tqtg added feature New feature/enhancement request models New models, changes to models labels Dec 27, 2023
@flywithu flywithu deleted the myrecvae2 branch January 3, 2024 13:46
@darrylong darrylong removed the feature New feature/enhancement request label Jan 11, 2024
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.

4 participants