madeleineudell / LowRankModels.jl

LowRankModels.jl is a julia package for modeling and fitting generalized low rank models.
Other
190 stars 65 forks source link

documentation needs update / OrdinalHinge constructor #49

Closed mravanba closed 8 years ago

mravanba commented 8 years ago

1) In the losses.jl, LogisticLoss is called LogLoss, which is inconsistent with the documentation. 2) Using OrdinalHinge loss will raise an error, because it does not define an empty constructor, needed by the copy method in utilities.

ahwillia commented 8 years ago

My preference would be for LogisticLoss over LogLoss as this is consistent with what is in EmpiricalRisks.jl and doesn't have the possibility of getting confused with the log function (e.g. as a canonical link for Poisson) On Jan 26, 2016 9:06 AM, "Siamak Ravanbakhsh" notifications@github.com wrote:

1) In the lossesjl, LogisticLoss is called LogLoss, which is inconsistent with the documentation 2) Using OrdinalHinge loss will raise an error, because it does not define an empty constructor, needed by the copy method in utilities

— Reply to this email directly or view it on GitHub https://github.com/madeleineudell/LowRankModels.jl/issues/49.

madeleineudell commented 8 years ago

Yes, I also prefer the LogisticLoss as a name. But where in losses.jl do you see LogLoss used?

madeleineudell commented 8 years ago

(I've fixed the other error you point out in the most recent commit.)

mravanba commented 8 years ago

hmm... you're right about the LogLoss. My local version was not up to date.