search

JuliaLinearAlgebra / NonNegLeastSquares.jl

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

Pull request/3a13ed96 #6

Closed silenj closed 7 years ago

silenj commented 7 years ago

Purpose: get rid of warnings, make the tests pass and make the code run on julia v0.5 and v0.6. I am not fluent in Julia yet so to be sure, pleas look at the code.

ahwillia commented 7 years ago

Thanks for the effort on cleaning this up. Before reviewing, I'd like to merge #3, which should also address many of these problems - @rdeits is there a timeframe for when that will be ready?

rdeits commented 7 years ago

Ah, sorry! #3 should be ready for review now

silenj commented 7 years ago

Hello,

A slow learner as I am, realized that most of my challenges are due to git and its behavior. Therefore I propose you skip my contribution at this stage. There are several inconsistencies in them which are there because I used them to pinpoint the errors. I will clean up this and learn how to use git properly... Sorry for the mess.

What breaks the testing routines is the behaviour of constants in Float32 or Float64. Therefore in the testing algorithms snippets from test/alspgrad.jl should be like this:

for T in (Float64, Float32) Wg = max.(rand(T, p, k) .- T(0.3), 0) Hg = max.(rand(T, k, n) .- T(0.3), 0) ....

end

For compatibility of julia 0.5 and julia 0.6 for the inner constructors I do not know how to proceed.

Regards Johan

On 13.04.2017 20:45, Alex Williams wrote:

Thanks for the effort on cleaning this up. Before reviewing, I'd like to merge #3 https://github.com/ahwillia/NonNegLeastSquares.jl/pull/3, which should also address many of these problems - @rdeits https://github.com/rdeits is there a timeframe for when that will be ready?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ahwillia/NonNegLeastSquares.jl/pull/6#issuecomment-293972250, or mute the thread https://github.com/notifications/unsubscribe-auth/AKJGU2_0wBvCjGias7BjlkR3VtapouN7ks5rvl8pgaJpZM4M8hAd.

rdeits commented 7 years ago

Getting inner constructors working properly in v0.5 and v0.6 requires a slightly different syntax. Check out: https://discourse.julialang.org/t/how-to-make-new-inner-constructor-syntax-compatible-with-0-5/2094 and https://github.com/JuliaLang/Compat.jl/issues/332