DOC modify plot_scalable_poly_kernels.py format #23009
Conversation
Corrected a typo in the original file. It read ``featrues`` instead of ``features``
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
…b.com:sveneschlbeck/scikit-learn into plot_scalable_poly_kernels_speedup
… Adjust spacing and fix a typo.
…at_plot_scalable_poly_kernels
…at_plot_scalable_poly_kernels
…at_plot_scalable_poly_kernels
| @@ -98,6 +114,9 @@ | |||
| # (`n_runs` = 1) in this example, in practice one should repeat the experiment several | |||
| # times to compensate for the stochastic nature of :class:`PolynomialCountSketch`. | |||
|
|
|||
| from sklearn.pipeline import Pipeline | |||
There was a problem hiding this comment.
Can we also use make_pipeline here and save some lines?
There was a problem hiding this comment.
This is a good point! I have replaced the use of Pipeline with make_pipeline, as you suggest.
There was a problem hiding this comment.
You are still using Pipeline, maybe you have not pushed your commit?
There was a problem hiding this comment.
Ah, my mistake. Thank you for letting me know! The change should be reflected correctly now.
| # Finally, plot the results of the different methods against their training | ||
| # times. As we can see, the kernelized SVM achieves a higher accuracy, | ||
| # but its training time is much larger and, most importantly, will grow | ||
| # much faster if the number of training samples increases. | ||
|
|
||
| import matplotlib.pyplot as plt | ||
|
|
There was a problem hiding this comment.
Maybe we can add a title to the plot below. Something similar to "Accuracy vs training time trade-off", what do you think?
There was a problem hiding this comment.
I am not the biggest fan of plot titles (generally) because I find the information they add is often redundant and they tend to harm the data-ink ratio. Perhaps a caption could be added to briefly summarize the figure, but I don't believe it's strictly necessary here.
If there is some merit to adding a title that I have overlooked, or if you think adding a caption would improve the figure, I am open to making a revision here.
There was a problem hiding this comment.
I don't have a strong opinion about adding a title, as I agree the information is already there. Thanks @jsilke!
…at_plot_scalable_poly_kernels
…at_plot_scalable_poly_kernels
|
Thanks the rendered artifacts look good, merging! Thanks also @ArturoAmorQ for the review! |
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Reference Issues/PRs
#22903
What does this implement/fix? Explain your changes.
Modifies plot_scalable_poly_kernels.py pursuant to the discussion in #22903:
Any other comments?
timecould also be swapped out forperf_counter, but I don't think it's necessary here.