Skip to content

jit all the functions which are slow #98

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

Open
shreyasbapat opened this issue Mar 7, 2019 · 19 comments
Open

jit all the functions which are slow #98

shreyasbapat opened this issue Mar 7, 2019 · 19 comments
Labels

Comments

@shreyasbapat
Copy link
Member

shreyasbapat commented Mar 7, 2019

🐞 Problem

Some methods are tooo slow! For which we brought einsteinpy.ijit.jit
🎯 Goal
Make einsteinpy fast!

💡 Possible solutions
Identify the slow methods, and

from einsteinpy.ijit import jit

@jit
def ..

📋 Steps to solve the problem

  • Comment below about what you've started working on.
  • Add, commit, push your changes
  • Submit a pull request and add this in comments - Addresses #<put issue number here>
  • Ask for a review in comments section of pull request
  • Celebrate your contribution to this project 🎉

P.S : See #98 (comment)

@Tushar-Tyagi
Copy link
Contributor

@shreyasbapat I would like to work on this issue.

@shreyasbapat
Copy link
Member Author

Sure! Go ahead. Its a little tricky one though :)

@Tushar-Tyagi
Copy link
Contributor

Any suggestions on how to identify the slow methords?

@ritzvik
Copy link
Member

ritzvik commented Feb 8, 2020

The only problem in jitting functions is njit does not support numpy array deceleration inside the function.
See this for more details.

If there is some solution to be found, try this on christoffels() in utils/kerr_utils.py.
Let's see what happens next.

@SuyashSalampuria
Copy link
Contributor

I would like to take up this issue @ritzvik @shreyasbapat

@shreyasbapat
Copy link
Member Author

@SuyashSalampuria One good first issue is enough. You can dive into more complicated issues now :)

@Starninja666
Copy link
Contributor

Hey, I'm looking out for some slow methods right now, interested in solving this issue once I find them.

@shreyasbapat
Copy link
Member Author

Sure. Will wait for a Pull Request

@Atheriz-46
Copy link

I would like to work on this.

@JeS24
Copy link
Member

JeS24 commented Jan 20, 2021

@Atheriz-46 Sure, but as mentioned by others, this issue is tricky due to the way numba and python work.

@ghost
Copy link

ghost commented Aug 7, 2023

As this is still open, can i work on this?

@JeS24
Copy link
Member

JeS24 commented Aug 7, 2023

@sanju50201 It is open. Feel free.

@ghost
Copy link

ghost commented Aug 8, 2023

using the JIT wrapper does increase the performance by few milli seconds, can i create a PR for the changes made?

@JeS24
Copy link
Member

JeS24 commented Aug 8, 2023

using the JIT wrapper does increase the performance by few milli seconds, can i create a PR for the changes made?

@sanju50201 Sure. Please also include a comparison table for the performance, something like With-jit | Without-jit.

@ghost
Copy link

ghost commented Aug 8, 2023

this is with the jit wrapper:
437534 function calls (421186 primitive calls) in 1.356 seconds

works well when i just do tasks individually with jit and without 'jit', however i think the compiler limits itself from interacting with the instances

@JeS24
Copy link
Member

JeS24 commented Aug 8, 2023

however i think the compiler limits itself from interacting with the instances

@sanju50201 Could you expand on this? Are you facing any errors?

@ghost
Copy link

ghost commented Aug 8, 2023

however i think the compiler limits itself from interacting with the instances

@sanju50201 Could you expand on this? Are you facing any errors?

Yes, this is the issue i'm facing:

numba.core.errors.TypingError: Failed in nopython mode pipeline (step: nopython frontend)
Untyped global name 'CartesianConversion': Cannot determine Numba type of <class 'type'>

@JeS24
Copy link
Member

JeS24 commented Aug 8, 2023

@sanju50201 Yeah, numba is quite limited when it comes to custom types (classes). See here & here.

However, in this particular case, you do not need to jit CartesianConversion (or other classes in the module), as the underlying functions are already using jit. See https://github.com/einsteinpy/einsteinpy/blob/main/src/einsteinpy/coordinates/utils.py.

You can perhaps experiment with the geodesics or the integrators modules, that aren't numba-accelerated. Note that the classes or their methods are usually incompatible with numba, even with type annotations. Try jit-ing the functions in the corresponding utils.py modules, as with the coordinates module above.

@ghost
Copy link

ghost commented Aug 8, 2023

@sanju50201 Yeah, numba is quite limited when it comes to custom types (classes). See here & here.

However, in this particular case, you do not need to jit CartesianConversion (or other classes in the module), as the underlying functions are already using jit. See https://github.com/einsteinpy/einsteinpy/blob/main/src/einsteinpy/coordinates/utils.py.

You can perhaps experiment with the geodesics or the integrators modules, that aren't numba-accelerated. Note that the classes or their methods are usually incompatible with numba, even with type annotations. Try jit-ing the functions in the corresponding utils.py modules, as with the coordinates module above.

@JeS24 This gives me a clear picture, thank you
let me see on the other things you mentioned

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
7 participants