Update to Optim@2

[devmotion]

Fixes #177.

[ablaom]

Thanks @devmotion for drafting this fix - very much appreciated.

Current fail is:

HuberReg: Test Failed at /home/runner/work/MLJLinearModels.jl/MLJLinearModels.jl/test/fit/robust.jl:57
    Expression: isapprox(J(θ_newton), 6.200183, rtol = 1.0e-5)
     Evaluated: isapprox(6.200521473589882, 6.200183; rtol = 1.0e-5)

The relevant test code is here

I can't really comment here as I don't understand where the reference value "6.200183" is coming from. There is a code comment that says "in these cases, if available, θ_newton is used as reference" but I don't follow, as θ_newton is computed but not apparently not used as the reference value. So where exactly does ""6.200183" come from?

@tlienart Great if you could clarify here.

[tlienart]

these values were reference values taken from fixed runs from sklearn, you can safely adjust the relative tolerance, I did have to play a bit with tolerances back in the days, effectively the aim here is to have something that's "close enough" but it doesn't have to be identical because the approaches are not the same.

[ablaom]

@tlienart Thanks for that lightning response and clarification.

@devmotion I think you can go ahead and relax the tolerance. Can you also please add a code comment top of file to the effect that the reference values come from sklearn?

[devmotion]

I updated the comment and the tests

[tlienart]

On second thoughts it might not have been a sklearn comparison but an optimizer comparison (it's been a while) but the conclusion is identical.

Thanks for your work guys!

[ablaom]

On second thoughts it might not have been a sklearn comparison but an optimizer comparison

@tlienart Could you please elaborate? I still don't understand what exactly you mean here.

[tlienart]

@ablaom I'm sorry I missed that message

it's a bit far from me but I recall testing several optimizers and wanting to check that the results are "close enough" (since they're all supposed to converge to a similar spot when the function is convex). This J(θ_newton) is the Jacobian taken at a given parameter obtained by using the newton optimization method (iirc the expensive one where you build and invert the hessian). The comparison number on the other side of the isapprox is a reference number which I don't recall right now how it got obtained but is either

• a reference number obtained from scikitlearn and hardcoded to avoid issues with versions or random seeds or whatever (still bad practice though)
• a refernce number obtained from another method

the general idea remaining the same: when the optimization function is convex and well behaved, you're supposed to land around the same place with a similar loss function. So getting there "close enough" is the test passing which is what you hit here.

I hope this clarifies it a bit, I'd actually argue you could deprecate all this "fixed" testing but when I was originally working on it, it made sense to compare against sklearn which is established and mature and which I took as an extra way to check for correctness.

[ablaom]

@tlienart Thanks for the further clarification.