Skip to content
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

speeding up python #642

Closed
Higginbottom opened this issue Nov 19, 2019 · 3 comments
Closed

speeding up python #642

Higginbottom opened this issue Nov 19, 2019 · 3 comments

Comments

@Higginbottom
Copy link
Collaborator

Just a quick issue to document attempts to try and speed up python (partly driven by the rad hydro models).

KSL has run regression tests, showing that a very significant amount of time is spent in radiation, (and a surprising amount in the subroutine den_config)

There is apparently not much fat in the routine - this is evidenced by the amount of time spent in den_config which literally just indexes things. Trying to time the routine reveals that a matter of microseconds is spent in each call - however many millions of calls can be made.

The main loop is to calculate the bound free opacity - which means for every photon, loops are made over all ions/cross sections, which can be hundreds.

Initial thinking by NSH (and agreed by KSL) was that we could limit the number of cross sections being calculated by simply not computing cross sections for any ion which has been set to the floor. MIN_DENSITY.

However, NSH has been poking around and realises that there is a very nice array already calculated - kbf_need which attempts to limit the number of cross sections to be used in calulate_ds by looking at the peak opacity in each cell for each cross section. If the opacity is less than some value then that cross section is not used in the calculate_ds routine.

This is why calculate ds is fast and resonate is slow despite them doing much the same as each other! One could argue that we should not be including cross sections in the radiation bit of the code that arent included in the calculate_ds - os the obvious idea is to use kbf_use in radiation too....

@jhmatthews
Copy link
Collaborator

sounds sensible to me!

@Higginbottom
Copy link
Collaborator Author

Hmmmmm, initial test simply using kbf_need for the loop in radiation gives very different results for regression tests - much faster tho.
This is a little worrying - after all if the list of BF cross sections in kbf_need is insufficient to get the important heating rates correct, then surely the opacity in calc_ds is likely to be underestimated. Hmmmm.

@Higginbottom
Copy link
Collaborator Author

I have done some work on this - and TBH, the only real game in town was to reduce the BF list used in the loop in radiation. My investigations using the regression test showed that if I reduced the list (by whatever method) to significantly speed up the code - I got very different regression results. And in this case, I still only got about 20% speedup - in the ionization cycles.
So, I propose closing this issue - its not possible to get a quick win here...

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

No branches or pull requests

2 participants