ibayer / fastFM

fastFM: A Library for Factorization Machines
http://ibayer.github.io/fastFM
Other
1.07k stars 206 forks source link

add n_more_iter option to `als.FMClassification.fit` #111

Closed lucidfrontier45 closed 7 years ago

lucidfrontier45 commented 7 years ago

I added n_more_iter option to als.FMClassification.fit just like als.FMRegression.fit.

I don't know if there is any theoretical incompatibility between classification problem and warm start and can't confirm due to this problem https://github.com/ibayer/fastFM-core/issues/25 .

If there is no incompatibility, please merge it.

ibayer commented 7 years ago

Thanks for the PR.

I don't see a theoretical issue. We need tests to ensure that we get exactly the same results with n_more_iter (warm start) and without. Below are examples how this as been done for the regression case.

https://github.com/ibayer/fastFM/blob/master/fastFM/tests/test_als.py#L96 https://github.com/ibayer/fastFM/blob/master/fastFM/tests/test_als.py#L125

lucidfrontier45 commented 7 years ago

@ibayer

OK. I'll add tests as soon as I resolve the fastfm-core build issue.

lucidfrontier45 commented 7 years ago

@ibayer added unittest

ibayer commented 7 years ago

@lucidfrontier45 Thanks for your contribution!