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.

Sign up for GitHub

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

Jump to bottom

Matrix factorization #729

Merged
merged 15 commits into from
Mar 15, 2019
Merged

Matrix factorization #729

merged 15 commits into from
Mar 15, 2019

Conversation

sh1ng
Copy link
Contributor

@sh1ng sh1ng commented Mar 5, 2019 •
edited
Loading

  • Based on https://github.com/cuMF/cumf_als
  • GPU only version. Do we need implement CPU version?
  • The code is quite awkward, I've started to make it more structured, but not finished
  • Only support float32 for now
  • Only single GPU for now
  • A few more MUST TO HAVE features(like releasing memory, more granular python API) are in progress and will be in next commits
  • There're a few copies of train dataset on GPU(CSC, CSR, COO sparse matrices), I'm going to fix it in a separate PR

But review can be started.

@sh1ng sh1ng requested a review from pseudotensor March 5, 2019 18:37
@sh1ng sh1ng added this to the 0.4.0 milestone Mar 5, 2019
@sh1ng sh1ng self-assigned this Mar 5, 2019
pseudotensor
pseudotensor previously approved these changes Mar 5, 2019
View reviewed changes
Copy link
Collaborator

@pseudotensor pseudotensor left a comment

Choose a reason for hiding this comment

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

Thanks for pulling this in!

pseudotensor
pseudotensor previously approved these changes Mar 8, 2019
View reviewed changes
Copy link
Collaborator

@pseudotensor pseudotensor left a comment

Choose a reason for hiding this comment

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

great, thanks.

sh1ng added 4 commits March 9, 2019 11:25
@sh1ng
initial version of matrix factorization
81f5843
@sh1ng
fix pylint
bc33dcc
@sh1ng
add test_10M.py
6b562fb
@sh1ng
A few more fixes:
a1607de 
* freeing memory
* copy X, theta back to host
* separate method to get factorization score
* pytest
@sh1ng sh1ng force-pushed the matrix_factorization branch from 87efdd0 to a1607de Compare March 9, 2019 10:26
@sh1ng
Copy link
Contributor Author

sh1ng commented Mar 15, 2019

The implementation is unstable when use LU solver instead CG

#define USE_CG // comment it out

But as CG is considered as a better alternative than "exact" solution, it's ok for now.

@sh1ng sh1ng mentioned this pull request Mar 15, 2019
Open
@sh1ng sh1ng requested a review from pseudotensor March 15, 2019 16:12
@sh1ng sh1ng mentioned this pull request Mar 15, 2019
Open
This was referenced Mar 15, 2019
Open
Open
Open
@sh1ng sh1ng merged commit 5ca02f5 into master Mar 15, 2019
@sh1ng sh1ng deleted the matrix_factorization branch March 15, 2019 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mdymczyk mdymczyk Awaiting requested review from mdymczyk

@navdeep-G navdeep-G Awaiting requested review from navdeep-G

@pseudotensor pseudotensor Awaiting requested review from pseudotensor

Assignees

@sh1ng sh1ng

Labels
factorization
Projects
None yet
Milestone
0.3.2
Development

Successfully merging this pull request may close these issues.
2 participants
@sh1ng @pseudotensor