Increase execution speed of plot_sparse_logistic_regression_20newsgroup.py #21619
Conversation
…xecution speed
| @@ -82,6 +82,7 @@ | |||
| penalty="l1", | |||
| max_iter=this_max_iter, | |||
| random_state=42, | |||
| warm_start=True, | |||
There was a problem hiding this comment.
you sure this has an effect? I'm not sure if in this case it should
There was a problem hiding this comment.
I am not so deep in how exactly they run these modules/their tests but this would have an influence depending on how often they are executed and in what order (e.g. multiple runs)
There was a problem hiding this comment.
Since you are creating a new lr instance each time you call fit, it should not have any impact.
warm_start=True could have an impact if you move the creation of the lr instance before entering the for this_max_iter in model_params["iters"] loop and then only calling the lr.set_params(max_iter=this_max_iter) inside the inner loop prior to calling fit. However wall time measurements would get a different meaning and would need to be readjusted before plotting. I have the feeling that this be too tricky to achieve or would render the example too complex.
Let's just use the training set size reduction and revert the use of warm_start=True.
| @@ -40,7 +40,7 @@ | |||
| solver = "saga" | |||
|
|
|||
| # Turn down for faster run time | |||
| n_samples = 10000 | |||
| n_samples = 7000 | |||
There was a problem hiding this comment.
I'd even go lower, maybe 4000 would still be enough?
There was a problem hiding this comment.
I'll look into it, probably 4000 is still enough :)
There was a problem hiding this comment.
Yes, time for second example is now down to 5 sec (from 9) but accuracy also dropped a bit (from 0.7 to 0.6). Will check out 5000 and 6000
There was a problem hiding this comment.
Okay, 5000 seems to be a good compromise, time is down to under 7 sec with accurace close to where it was before
removed ``warm_start=True``
|
Thank you for working on this PR! The proposed change looks to already be on which was merged in #21773. With that in mind, I am closing this PR. |
#21598 @sply88 @adrinjalali @cakiki
Adapted the number of samples and added the
warm_startoption for the Logistic Regression Modell. Execution time was reduced from 45 sec to 9 sec :