FIX for SAG with sparse samples.

[fabianp]

The problem was that when the solution was updated just in time
the different scaling accumulated were not considered. They were
treated as if they had been constant in the last iterations.

This should fix issue #33 , although because of some python 3 incompatibility I've not yet run the full test suite.

[fabianp]

The drawback of this solution is that there is a need for an extra n_samples array, but I don't see any other solution. Note that the scikit-learn version uses a similar strategy.

[mblondel]

The drawback of this solution is that there is a need for an extra n_samples array

This is not big deal.

Thanks for the fix. Tell me when you run the tests.

[mblondel]

Could you add a small test which does the following: run the algorithm on the same data, once with a numpy array and once with a sparse matrix and check that the learned coefficients are the same. The data should include some zero features.

[fabianp]

This still needs some work, I realised with the tests that the problem persists for big alphas (alpha > 10).

[mblondel]

@TomDLT any idea?

[fabianp]

Just realized that the same change needs to be done in the finalize block. I'll try it ASAP

[TomDLT]

I mentioned this problem in scikit-learn's SAG PR, and this PR seems to fix it.

Just realized that the same change needs to be done in the finalize block

Correct.

[mblondel]

@TomDLT Fabian will add SAGA to lightning, which is why we need to fix SAG first.

[fabianp]

Changed that and now the tests pass. It should be fixed.

On a related notice, I noticed that violation_init can be zero and so violation_ratio = violation / violation_init is undefined. There's no exception because of the cython: cdivision=True. I don't know the logic to enough to see what is the best way to deal with this. @mblondel ?

[mblondel]

The computation of the violation measure could be wrong too but it's difficult for me to check right now since I am at a conference. Basically the violation measure is just the l2 norm of the gradient. Could you check if this is actually what is being computed?

[fabianp]

no problem, I'll look into it.

[fabianp]

I solved this by a distinction of cases when violation == 0. This way there is no division by zero.

It now looks good to me.

[mblondel]

Thanks, merging.

[mblondel]

I made a small cosmit in master.