Skip to content

Replace %time with %timeit #206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
HumphreyYang opened this issue Apr 2, 2025 · 4 comments
Closed

Replace %time with %timeit #206

HumphreyYang opened this issue Apr 2, 2025 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@HumphreyYang
Copy link
Collaborator

@shizejin reported that there is a misleading comparison in the introduction to JAX:

Image

It might relates to the timing magic we used. %time is sometimes influenced other factors such as garbage collections, workload etc:
https://stackoverflow.com/questions/17579357/time-time-vs-timeit-timeit

I think %timeit might be a more robust way to compare the results, which is also used in JAX documents:
https://docs.jax.dev/en/latest/quickstart.html

The result under %timeit can be found here:
Image

@HumphreyYang HumphreyYang added the enhancement New feature or request label Apr 2, 2025
@jstac
Copy link
Contributor

jstac commented Apr 7, 2025

Thanks @HumphreyYang and thanks @shizejin for reporting!

(@HumphreyYang always puts in great PRs, so the whole issue is clear at a glance. This is super valuable to me.)

I was trying to avoid timeit because people who don't understand ipython get the impression that the code is slow.

But it seems this is causing problems.

@HumphreyYang , let's do as you recommend. Would you mind to put the fix in?

@HumphreyYang
Copy link
Collaborator Author

Many thanks @jstac for the suggestion! I will submit a FIX for this.

I was trying to avoid timeit because people who don't understand ipython get the impression that the code is slow.

I think this is a very valid concern as well. The reader sees that the number is small but notices that the code takes longer to run.

I will add a brief note below the first use of %timeit, explaining that it runs the line multiple times to provide a robust comparison.

Given this concern, should we change %timeit globally or only in the introductory lecture?

I think the issue is less noticeable when the runtime differences are large.

@HumphreyYang HumphreyYang self-assigned this Apr 7, 2025
@jstac
Copy link
Contributor

jstac commented Apr 7, 2025

I agree @HumphreyYang . Let's change it only in the introductory lecture and with the note that you mention. Thanks.

@HumphreyYang
Copy link
Collaborator Author

Fixed by using JAX==0.6.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants