/cc @OmarManzoor.

Hi @ogrisel ,
I was having a look at this and it seems that the benchmark uses a dtype of float32 by default. Is it possible that with float32 more iterations are taken to converge to a similar result as with float64?

Hi @ogrisel , I was having a look at this and it seems that the benchmark uses a dtype of float32 by default. Is it possible that with float32 more iterations are taken to converge to a similar result as with float64?

But then the dense case is not impacted and that uses float32 as well. So this might not be the case.

Indeed it could be. Can you check the final n_iter_ value in both cases?

I removed the caching part from _synth_regression_sparse_dataset and used about 10 iterations to account for randomness. This is the code I used https://gist.github.com/OmarManzoor/ba85c684991ba132fbf544f0433c2bfa

Sparse, float64 - Total number of iterations: [85, 84, 57, 76, 84, 65, 96, 81, 81, 93]
Sparse, float32 - Total number of iterations: [1000, 960, 1000, 1000, 835, 1000, 839, 1000, 1000, 1000]

Ouch. This does not look good. I didn't look at the script yet but it shows that we never converge most of the time. It might be interesting to have a look at the loss overtime.

Oh, I see that you put a tolerance tol=1e-16. This cannot work with 32 bits because the smallest tolerance would be around 1e-7.

OK I think the issue is our benchmark here:

The tolerance is too small for the 32 bits implementation. I don't know if tol=1e-7 would be a good default there.

OK I think the issue is our benchmark here:

scikit-learn/asv_benchmarks/benchmarks/linear_model.py

Line 167 in 10eaa52

estimator = SGDRegressor(max_iter=1000, tol=1e-16, random_state=0)

The tolerance is too small for the 32 bits implementation. I don't know if tol=1e-7 would be a good default there.

I see. So should we change this value for the 32 bit case?

I would ask for advise from @jeremiedbb and @ogrisel to know what they think. I assume that 1e-16 was to get a model that converged. So I don't know if we should have a tolerance that depends on the dtype or a common tolerance that works for both dtypes.

I would ask for advise from @jeremiedbb and @ogrisel to know what they think. I assume that 1e-16 was to get a model that converged. So I don't know if we should have a tolerance that depends on the dtype or a common tolerance that works for both dtypes.

Understood.

Thanks for the investigation @OmarManzoor and @glemaitre!

I think we could set tol to 1e-7 for both dtypes. WDYT @jeremiedbb?

I wonder why it works with a low tol value for dense data though.

I wonder why it works with a low tol value for dense data though.

Could it have to do with the size of the data. The dense dataset only has 200 features.

Oh, I see that you put a tolerance tol=1e-16. This cannot work with 32 bits because the smallest tolerance would be around 1e-7.

There's a confusion here. 1e-16 is representable in float32, and you can check that a number is smaller than that. Machine precision represents the max relative difference between 2 consecutive representable numbers.

If the stopping criterion of sgd was if (new_score / prev_score) < tol, 1e-16 would be fine.
However the current stopping criterion is if score < best_score + tol, In that case, as long as tol < best_score * float32.eps, the tol is rounded out and the expression becomes ìf score < best_score.

In the benchmaks the goal is to always perform the max_iter iterations, So probably setting tol=None would be more appropriate.

and catch ConvergenceWarning and assert that model.n_iter_ == model.max_iter after calling fit to make that explicit.