search

JuliaLinearAlgebra / NonNegLeastSquares.jl

Some nonnegative least squares solvers in Julia
MIT License
46 stars 11 forks source link

Not perfect but worth caching somewhere #30

Closed caseykneale closed 3 years ago

caseykneale commented 3 years ago

Some of the test cases for the CD algorithm don't match with say scikit learns NNLS(and by transitive property the ones in NonNegLeastSquares.jl). Some do. I tried to bring ADMM back to life, but I don't have a lot of time to flesh that out. Some of the syntax from before Julia 0.7 was upgraded but there's an error there.

Don't have time to do serious digging here. My guess is the CD algorithm is caught in some local min's. Likely the CD algorithm more closely matches ADMM and not NNLS/activeset type methods. Maybe just pop this on another branch for now? Sorry crazy swamped right now.

JeffFessler commented 3 years ago

Thanks for the PR. I might look at it over the summer (after semester ends) if no one else gets to it first. Depending on the data, NNLS might have multiple global minimizers so we might need to check the cost function not just the parameter estimates.

JeffFessler commented 3 years ago

btw, i created a new "cd" branch but i don't know how to put your PR into that branch...

MatFi commented 3 years ago

Merged the fork with the cd repo in https://github.com/ahwillia/NonNegLeastSquares.jl/commit/71527fa29669e6acff97759f8a2b3bdbb0b5ae1a

just for future reference on how to merge two different remotes (as in here)

git checkout cd
git remote add cdfork https://github.com/caseykneale/NonNegLeastSquares.jl     
git pull cdfork master
git push origin

EDIT: Better just to change the base branch using the top right edit button and merge it here. (I've undone my changes)

caseykneale commented 3 years ago

yea if this sits for a few weeks I can likely take a crack at this again. What I really need to do is find what paper I got the algorithm from. Otherwise I'll have to unpack it for what it is and yea double check it.