EUREC4A-UK / lagtraj

Python trajectory code for Lagrangian simulations
MIT License
11 stars 9 forks source link

suggestion: install and use numba by default #162

Closed leifdenby closed 2 years ago

leifdenby commented 2 years ago

@sjboeing I just installed lagtraj on a new computer I am developing on, and I noticed the tests were running very slowly. I then realise that this is because we don't install and use numba by default. I think we should change this as people are unlikely to realise that they should install numba otherwise. The only reasons I can see for not doing this are either 1) numba is hard to install or 2) numba isn't supported on all platforms we want to target. I don't think either of these are true:

  1. numba can be installed with pip, conda isn't required
  2. I can't see a platform missing from numba's list of supported platforms: https://numba.pydata.org/numba-doc/latest/user/installing.html

What do you think?

sjboeing commented 2 years ago

Yes, I think by now numba is only a minor additional dependency. It would be good to keep the option of running without numba optional (but maybe the user should need to very explicitly request it), as it can be useful for debugging.

leifdenby commented 2 years ago

I've made a pull-request to do this. I think we should also remove the using numba print that happens when we run every lagtraj command. It shouldn't matter to the user that they're using numba, they don't need to know about that. Instead I would like to warn the user if numba isn't installed.

In the situation where one is debugging (i.e. someone developing the code) they can simply uninstall numba or remove the njit decorator temporarily.

leifdenby commented 2 years ago

I'm doing this now because I've just realised that we've been running on the CI tests without numba which is a huge waste of time and cpu cycles

leifdenby commented 2 years ago

The CI test just completed in 7 minutes rather than over half an hour

leifdenby commented 2 years ago

https://github.com/EUREC4A-UK/lagtraj/pull/167 is ready for review now @sjboeing when you've got a moment to have a look :)

sjboeing commented 2 years ago

Closing via #167