Plasma optimization #399

wkerzendorf · 2015-08-25T16:22:29Z

rewrite of some functions to make them faster. There are still some functions to go and specifically one function is already in C/Cython and would need to be sped-up there.

wkerzendorf · 2015-08-25T16:24:23Z

@unoebauer this is now a factor of 3. One function is now in C and takes up a significant fraction of the time. The other slower function is the random blackbody nu function, the reimplementation would probably significantly speed that up (currently 1/3 of the non-montecarlo functions).

@mreinecke I'll mark a function that is cython (so very close to C). but that can probably be sped up by openmp (array operations).

wkerzendorf · 2015-08-25T16:25:06Z

tardis/plasma/properties/util/macro_atom.pyx

+                p_transition[j, k] /= norm_factor[k]
+
+
+def calculate_transition_probabilities(


@mreinecke this is the beast. You could just write it in C and maybe with openmp it would be faster. I'm wondering if memory bandwidth is an issue here.

On 08/25/15 18:25, Wolfgang Kerzendorf wrote:

cdef end_id = 0

for i in range(len(reference_levels) - 1):

norm_factor[:] = 0.0

for j in range(reference_levels[i], reference_levels[i + 1]):

for k in range(p_transition.shape[1]):

norm_factor[k] += p_transition[j, k]

for j in range(reference_levels[i], reference_levels[i + 1]):

for k in range(0, p_transition.shape[1]):

if norm_factor[k] == 0.0:

continue

p_transition[j, k] /= norm_factor[k]

+def calculate_transition_probabilities(

@mreinecke this is the beast. You could just write it in C and maybe with openmp it would be faster. I'm wondering if memory bandwidth is an issue here.

It may be possible to optimize this a bit, but there are a few issues to
consider first:

how large are the individual loop iteration counts (roughly, order of
magnitude is fine)? This determines the best arrangement of the nested
loops.

Is it known how the array p_transition is laid out in memory on the
Python side? We won't gain any performace by tweaking the routine as
long as the glue code between Python and C has to do things like array
copying and maybe even transposition. This is because this function is
absolutely dominated by memory accesses; the cost of arithmetic is
negligible in comparison.

related to the point above: does "p_transition[j, k]" in Cython mean
the same as "p_transition[j][k]" in C? More specifically, will
p_transition[j][k] and p_transition[j][k+1] be neighbours in memory?

Overall it would be beneficial if "p_transition" could be stored in a
way that elements with j and j+1 are neighbouring in memory, but I'm not
sure if the memory layout is constrained somehow by other considerations.

Cheers,
Martin

Martin - do you want to meet? maybe this is easier discussed in person.

On 08/26/15 10:44, Wolfgang Kerzendorf wrote:

cdef end_id = 0

for i in range(len(reference_levels) - 1):

norm_factor[:] = 0.0

for j in range(reference_levels[i], reference_levels[i + 1]):

for k in range(p_transition.shape[1]):

norm_factor[k] += p_transition[j, k]

for j in range(reference_levels[i], reference_levels[i + 1]):

for k in range(0, p_transition.shape[1]):

if norm_factor[k] == 0.0:

continue

p_transition[j, k] /= norm_factor[k]

+def calculate_transition_probabilities(

Martin - do you want to meet? maybe this is easier discussed in person.

Sure, but I have a few quick things to finish now. I'll let you know!

unoebauer · 2015-08-25T18:27:00Z

@wkerzendorf Travis fails because of a NameError related to jit. Maybe because you removed numba?

unoebauer · 2015-08-25T20:16:02Z

@wkerzendorf
So, I removed the last jist statement left in the PR. But running the tardis_example with this code version still fails, but now with an error in ion_population.py:

tardis.io.config_reader - INFO - Reading Atomic Data from kurucz_cd23_chianti_H_He.h5
tardis.atomic - INFO - Read Atom Data with UUID=5ca3035ca8b311e3bb684437e69d75d7 and MD5=21095dd25faa1683f4c90c911a00c3f8
tardis.io.config_reader - INFO - "initial_t_inner" is not specified in the plasma section - initializing to 9933.95199592 K with given luminosity
tardis.io.config_reader - WARNING - No "species" given - ignoring other NLTE options given:
{   'classical_nebular': False, 'coronal_approximation': False}
tardis.io.config_reader - WARNING - No convergence criteria selected - just damping by 0.5 for w, t_rad and t_inner
tardis.plasma.base - WARNING - dot2tex missing. Plasma graph will not be generated.
Traceback (most recent call last):
  File "/home/ulrich/python-virtualenv/tardis-devel/bin/tardis", line 4, in <module>
    __import__('pkg_resources').run_script('tardis-sn==1.0.1', 'tardis')
  File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 534, in run_script
    self.require(requires)[0].run_script(script_name, ns)
  File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 1438, in run_script
    execfile(script_filename, namespace, namespace)
  File "/home/ulrich/python-virtualenv/tardis-devel/lib/python2.7/site-packages/tardis_sn-1.0.1-py2.7-linux-x86_64.egg/EGG-INFO/scripts/tardis", line 63, in <module>
    radial1d_mdl = model.Radial1DModel(tardis_config)
  File "/home/ulrich/python-virtualenv/tardis-devel/local/lib/python2.7/site-packages/tardis_sn-1.0.1-py2.7-linux-x86_64.egg/tardis/model.py", line 132, in __init__
    link_t_rad_t_electron=0.9, helium_treatment=tardis_config.plasma.helium_treatment)
  File "/home/ulrich/python-virtualenv/tardis-devel/local/lib/python2.7/site-packages/tardis_sn-1.0.1-py2.7-linux-x86_64.egg/tardis/plasma/standard_plasmas.py", line 117, in __init__
    previous_beta_sobolevs=initial_beta_sobolevs)
  File "/home/ulrich/python-virtualenv/tardis-devel/local/lib/python2.7/site-packages/tardis_sn-1.0.1-py2.7-linux-x86_64.egg/tardis/plasma/base.py", line 23, in __init__
    self.update(**kwargs)
  File "/home/ulrich/python-virtualenv/tardis-devel/local/lib/python2.7/site-packages/tardis_sn-1.0.1-py2.7-linux-x86_64.egg/tardis/plasma/base.py", line 144, in update
    self.plasma_properties_dict[module_name].update()
  File "/home/ulrich/python-virtualenv/tardis-devel/local/lib/python2.7/site-packages/tardis_sn-1.0.1-py2.7-linux-x86_64.egg/tardis/plasma/properties/base.py", line 86, in update
    *self._get_input_values()))
  File "/home/ulrich/python-virtualenv/tardis-devel/local/lib/python2.7/site-packages/tardis_sn-1.0.1-py2.7-linux-x86_64.egg/tardis/plasma/properties/ion_population.py", line 62, in calculate
    self.phis[start_id - i:end_id - i - 1] = phis
ValueError: could not broadcast input array from shape (93,20) into shape (88,20)

wkerzendorf · 2015-08-26T08:34:26Z

@unoebauer - it works for me when running the tardis_example.yml now. but still struggling with the tests.

unoebauer · 2015-08-26T08:56:24Z

@wkerzendorf - still doesn't work for me. It still fails with the same shape mismatch error as before...

wkerzendorf · 2015-08-26T10:12:14Z

@unoebauer tardis_example?

wkerzendorf · 2015-08-26T10:12:35Z

@unoebauer sorry - I just see that it is. hmm.

unoebauer · 2015-08-26T15:08:22Z

@wkerzendorf Good news: the last commit fixed the convergence issue in the plasma part
Bad news: a simple tardis_example calculation still fails immediately with the shape mismatch error

unoebauer · 2015-08-26T15:19:32Z

Ok - the shape mismatch error may be connected to pandas. It occurred when using pandas 0.14.1 (debian jessie). After upgrading to pandas 0.16.2, tardis_example runs without problems

wkerzendorf · 2015-08-26T15:59:14Z

@unoebauer it works (well the coverage still sucks 😉 but I'll update this one a bit more)

aoifeboyle · 2015-09-06T10:19:52Z

When I run this I get a bunch of these errors in the initial iteration:
SettingWithCopyWarning: A value is trying to be set on a copy of a slice from a DataFrame.
Does anyone else?

wkerzendorf · 2015-09-07T11:40:12Z

@aoifeboyle I wonder if this is the problem that comes from not having it merged with the newest master. Can you locate it?

aoifeboyle · 2015-09-10T19:55:49Z

@wkerzendorf Could you merge this PR soonish?

Plasma optimisation fix.

wkerzendorf · 2015-09-24T16:09:41Z

@aoifeboyle are we ready to merge this.

aoifeboyle · 2015-09-25T08:50:32Z

@wkerzendorf Yes, sure.

Plasma optimization

wkerzendorf added 6 commits August 21, 2015 17:45

speeding up several functions by large factors

f2e0f17

mend

971b7c2

stuck at c array passing; making new PR

c67b928

Merge branch 'montecarlo/memoryview' into plasma/faster_faster

3d082ed

some speedup

77e5fa2

cleanup of several functions

8193ff4

wkerzendorf reviewed Aug 25, 2015
View reviewed changes

removed numba requirement

221b524

wkerzendorf added 2 commits August 26, 2015 10:26

took out the jit

4dad917

changed some of the plasma functions to work again

d21b56a

some fix

b170a2b

wkerzendorf added 4 commits August 26, 2015 12:21

Merge remote-tracking branch 'upstream/master' into plasma/faster_faster

16a70bd

added fix for the new PhiLTE optimization

810af59

removed debug statement

73128aa

restructured to allow the philte staticmethods to work somewhere else

87700b1

wkerzendorf mentioned this pull request Aug 26, 2015

Give each thread it's own mt_state #395

Closed

fixed some oversight

06abc15

added new requirement for pandas 0.16

c2282fb

Merge remote-tracking branch 'upstream/master' into plasma/faster_faster

e037392

wkerzendorf mentioned this pull request Aug 31, 2015

Organising of plasma base files. #407

Merged

removed graph

b99d706

Aoife Boyle added 2 commits September 7, 2015 21:02

Pandas error fix

95c0940

Update base.py

6b9150e

wkerzendorf added 4 commits September 15, 2015 14:53

Merge pull request #19 from aoifeboyle/pr/399

043cee0

Plasma optimisation fix.

trying to optimize the the stimulated emission

4112e46

some clean-up

2e5e9d8

Merge remote-tracking branch 'upstream/master' into plasma/faster_faster

5aeb2f8

aoifeboyle mentioned this pull request Sep 25, 2015

WIP: Cleaning up plasma #410

Merged

wkerzendorf added a commit that referenced this pull request Sep 25, 2015

Merge pull request #399 from wkerzendorf/plasma/faster_faster

e91c79c

Plasma optimization

wkerzendorf merged commit e91c79c into tardis-sn:master Sep 25, 2015

unoebauer mentioned this pull request Sep 28, 2015

Which Pandas version is required for new plasma framework? #415

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plasma optimization #399

Plasma optimization #399

wkerzendorf commented Aug 25, 2015

wkerzendorf commented Aug 25, 2015

wkerzendorf Aug 25, 2015

ghost Aug 26, 2015

wkerzendorf Aug 26, 2015

ghost Aug 26, 2015

unoebauer commented Aug 25, 2015

unoebauer commented Aug 25, 2015

wkerzendorf commented Aug 26, 2015

unoebauer commented Aug 26, 2015

wkerzendorf commented Aug 26, 2015

wkerzendorf commented Aug 26, 2015

unoebauer commented Aug 26, 2015

unoebauer commented Aug 26, 2015

wkerzendorf commented Aug 26, 2015

aoifeboyle commented Sep 6, 2015

wkerzendorf commented Sep 7, 2015

aoifeboyle commented Sep 10, 2015

wkerzendorf commented Sep 24, 2015

aoifeboyle commented Sep 25, 2015

		p_transition[j, k] /= norm_factor[k]


		def calculate_transition_probabilities(

Plasma optimization #399

Plasma optimization #399

Conversation

wkerzendorf commented Aug 25, 2015

wkerzendorf commented Aug 25, 2015

wkerzendorf Aug 25, 2015

Choose a reason for hiding this comment

ghost Aug 26, 2015

Choose a reason for hiding this comment

wkerzendorf Aug 26, 2015

Choose a reason for hiding this comment

ghost Aug 26, 2015

Choose a reason for hiding this comment

unoebauer commented Aug 25, 2015

unoebauer commented Aug 25, 2015

wkerzendorf commented Aug 26, 2015

unoebauer commented Aug 26, 2015

wkerzendorf commented Aug 26, 2015

wkerzendorf commented Aug 26, 2015

unoebauer commented Aug 26, 2015

unoebauer commented Aug 26, 2015

wkerzendorf commented Aug 26, 2015

aoifeboyle commented Sep 6, 2015

wkerzendorf commented Sep 7, 2015

aoifeboyle commented Sep 10, 2015

wkerzendorf commented Sep 24, 2015

aoifeboyle commented Sep 25, 2015