Faster LOS calc signal with new_calc = True by lasofivec · Pull Request #322 · ToFuProject/tofu

lasofivec · 2020-01-06T18:02:51Z

Same as #308 but for the new algorithm implemented (activated whent LOS_calc_signal(..., new_calc=True, ...)).

Here are the results with the same benchmark

In [1]: import tofu as tf
/home/ITER/mendozl/repositories/tofu/usr/lib/python3.6/site-packages/tofu-1.4.2a5-py3.6-linux-x86_64.egg/tofu/imas2tofu/__init__.py:83: UserWarning:
You do not seem to be using the latest IMAS version:
'module list' vs 'module av IMAS' suggests:
	- Current version: 3.24.0-4.1.5
	- Latest version : 3.25.0-4.4.0
  warnings.warn(msg)
/home/ITER/mendozl/repositories/tofu/usr/lib/python3.6/site-packages/tofu-1.4.2a5-py3.6-linux-x86_64.egg/tofu/__init__.py:95: UserWarning:
The following subpackages are not available:
    - tofu.mag
  => see tofu.dsub[<subpackage>] for details.
  warnings.warn(msg)

In [2]: multi = tf.imas2tofu.MultiIDSLoader(user='hoeneno', tokamak='convert', shot=134000, run=29, ids=['edge_sources'])
Getting ids   [occ]  tokamak  user     version  shot    run  refshot  refrun
------------  -----  -------  -------  -------  ------  ---  -------  ------
edge_sources  [0]    convert  hoeneno  3        134000  29   -1       -1
_d^[[A
In [3]: _dshort = {'core_sources': {'1drhotn':'source[identifier.name=radiation].profiles_1d[time].grid.rho_tor_norm',
   ...:                             '1deEnergy':'source[identifier.name=radiation].profiles_1d[time].electrons.energy'
   ...:                                  },
   ...:            'equilibrium': {'2dpsi': 'time_slice[time].profiles_2d[0].psi',
   ...:                            '2dmeshR': 'time_slice[time].profiles_2d[0].r',
   ...:                            '2dmeshZ': 'time_slice[time].profiles_2d[0].z'}}
   ...:

In [4]: multi.set_shortcuts(dshort=_dshort)

In [5]: plasma = multi.to_Plasma2D()

In [6]: cam = tf.load('/home/ITER/munechk/public/MyTofu/output/ITER_test_camera_config.npz')
Loaded from:
    /home/ITER/munechk/public/MyTofu/output/ITER_test_camera_config.npz

In [7]: %timeit sig_newbasic, units = cam.calc_signal_from_Plasma2D(plasma, quant='edge_sources.2dradiation', plot=False, res=0.1, newcalc=True, method='sum', minimize='calls')
6.79 s ± 116 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

In [8]: %timeit sig_newbasic, units = cam.calc_signal_from_Plasma2D(plasma, quant='edge_sources.2dradiation', plot=False, res=0.1, newcalc=False)
7.36 s ± 50.8 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

In [9]: %timeit  sig_oldufunc, units = cam.calc_signal_from_Plasma2D(plasma, quant='edge_sources.2dradiation', plot=False, res=0.1, newcalc=False, fill_value=0.)
7.25 s ± 40.5 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

In [10]: sig_newbasic, units = cam.calc_signal_from_Plasma2D(plasma, quant='edge_sources.2dradiation', plot=False, res=0.1, newcalc=True, method='sum', minimize='calls')

In [11]:  sig_oldufunc, units = cam.calc_signal_from_Plasma2D(plasma, quant='edge_sources.2dradiation', plot=False, res=0.1, newcalc=False, fill_value=0.)

In [12]: np.allclose(sig_newbasic.data, sig_oldufunc.data)
Out[12]: True

The difference is less obvious than in the OPR, but new_calc=True seems to always yield the best results.
(I tried the benchmark in the hpc-login02 cluster)

Didou09

Fine for me when tests are passing,
But there seems to be an issue with array contiguity in unit tests, do you see why ?

codecov-io · 2020-01-07T14:25:51Z

Codecov Report

Merging #322 into devel will decrease coverage by 0.76%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##            devel     #322      +/-   ##
==========================================
- Coverage   41.03%   40.26%   -0.77%     
==========================================
  Files          79       79              
  Lines       23472    26345    +2873     
==========================================
+ Hits         9632    10609     +977     
- Misses      13840    15736    +1896

Impacted Files	Coverage Δ
tofu/geom/_comp_optics.py	`10.28% <0%> (-2.55%)`	⬇️
tofu/geom/_plot_optics.py	`5.7% <0%> (-1.26%)`	⬇️
tofu/geom/_core_optics.py	`18.28% <0%> (-0.25%)`	⬇️
tofu/imas2tofu/_core.py	`0.52% <0%> (-0.2%)`	⬇️
tofu/geom/_core.py	`64.34% <0%> (+0.98%)`	⬆️
tofu/geom/utils.py	`48.59% <0%> (+5.38%)`	⬆️
tofu/utils.py	`54.26% <0%> (+6.27%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d7d5f1...e1f4838. Read the comment docs.

lasofivec · 2020-01-07T14:48:32Z

Not sure of what you mean by "re assigning" but, the problem came from the fact that the result stored in sig was C-contiguous while sig is explicitly defined as a Fortran contiguous array (faster computation for all other cases).
So converting to result to a F-contiguous array solves the problem: sig = np.asfortranarray(...

Didou09

Tests passing => nice

2 questions though on val_arr and val_2d (see comments)

Didou09 · 2020-01-07T15:14:41Z

            # ..................................................................
        if ani:
-            val_2d = func(pts, t=t, vect=-usbis, **fkwdargs)
+            val_arr = func(pts, t=t, vect=-usbis, **fkwdargs)


val_arr does not seem to have been declared, is it voluntary ?

we declare for speed ups, see my next comment. (Sorry some times I have to push in order to try it in the ITER clusters).

Didou09 · 2020-01-07T15:14:48Z

        else:
-            val_2d = func(pts, t=t, **fkwdargs)
+            val_arr = func(pts, t=t, **fkwdargs)
+        val_2d = val_arr


Why is val_2d still necessary if we use val_arr now ?

val_2d is a memory view. So computations and slicing in the other cases are much faster. val_arr is a numpy array, I thought this could be the origin of the errors in the unit tests (I tried different solutions).

I am benchmarking if there is a faster solution (between using only val_2d or using val_2d and val_arr).

ok, thanks, I was not sure

It looks like they take more or less the same time. I decided to only keep the memory view val_2d. I also tried to declare the variable to see if this helped, no visible changes. So I would leave the simplest solution (with only one variable)

lasofivec · 2020-01-07T15:56:56Z

Latest results:

In [1]: import tofu as tf
/home/ITER/mendozl/repositories/tofu/usr/lib/python3.6/site-packages/tofu-1.4.2a5-py3.6-linux-x86_64.egg/tofu/imas2tofu/__init__.py:83: UserWarning:
You do not seem to be using the latest IMAS version:
'module list' vs 'module av IMAS' suggests:
	- Current version: 3.24.0-4.1.5
	- Latest version : 3.25.0-4.4.0
  warnings.warn(msg)
/home/ITER/mendozl/repositories/tofu/usr/lib/python3.6/site-packages/tofu-1.4.2a5-py3.6-linux-x86_64.egg/tofu/__init__.py:95: UserWarning:
The following subpackages are not available:
    - tofu.mag
  => see tofu.dsub[<subpackage>] for details.
  warnings.warn(msg)

In [2]: multi = tf.imas2tofu.MultiIDSLoader(user='hoeneno', tokamak='convert', shot=134000, run=29, ids=['edge_sources'])
Getting ids   [occ]  tokamak  user     version  shot    run  refshot  refrun
------------  -----  -------  -------  -------  ------  ---  -------  ------
edge_sources  [0]    convert  hoeneno  3        134000  29   -1       -1

In [3]: _dshort = {'core_sources': {'1drhotn':'source[identifier.name=radiation].profiles_1d[time].grid.rho_tor_norm',
   ...:                             '1deEnergy':'source[identifier.name=radiation].profiles_1d[time].electrons.energy'
   ...:                                  },
   ...:            'equilibrium': {'2dpsi': 'time_slice[time].profiles_2d[0].psi',
   ...:                            '2dmeshR': 'time_slice[time].profiles_2d[0].r',
   ...:                            '2dmeshZ': 'time_slice[time].profiles_2d[0].z'}}
   ...:

In [4]: multi.set_shortcuts(dshort=_dshort)

In [5]: plasma = multi.to_Plasma2D()

In [6]: cam = tf.load('/home/ITER/munechk/public/MyTofu/output/ITER_test_camera_config.npz')
Loaded from:
    /home/ITER/munechk/public/MyTofu/output/ITER_test_camera_config.npz

In [7]: %timeit  sig_oldufunc, units = cam.calc_signal_from_Plasma2D(plasma, quant='edge_sources.2dradiation', plot=False, res=0.1, newcalc=False, fill_value=0.)
7.44 s ± 114 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

In [8]: %timeit sig_newbasic, units = cam.calc_signal_from_Plasma2D(plasma, quant='edge_sources.2dradiation', plot=False, res=0.1, newcalc=True, method='sum', minimize='calls')
6.73 s ± 102 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

lasofivec · 2020-01-07T15:59:58Z

If tests pass and you agree. I am ok for merging this PR

Didou09

Good for me !
Will merge when tests are passed,
May I suppress the branch ?

lasofivec · 2020-01-07T16:10:38Z

Yes :) it looks like everything is working !

lasofivec and others added 2 commits January 6, 2020 18:02

speeding up new calc

d1f438e

[enhacement] defined res and indices the right way, erased old code

70e63e0

lasofivec requested a review from Didou09 January 6, 2020 18:02

lasofivec self-assigned this Jan 6, 2020

lasofivec added enhancement geom - improve labels Jan 6, 2020

Didou09 requested changes Jan 7, 2020

View reviewed changes

fixed f-contiguous error

e332b2a

Didou09 reviewed Jan 7, 2020

View reviewed changes

only keeping memory view

e1f4838

Didou09 approved these changes Jan 7, 2020

View reviewed changes

Didou09 merged commit 64c398f into devel Jan 7, 2020

Didou09 deleted the Faster_LOSCalcSignalSumCalls_fork branch January 7, 2020 17:04

Didou09 mentioned this pull request Jan 30, 2020

tofu 1.4.2 preparation #339

Merged

Didou09 mentioned this pull request Mar 10, 2020

Preparing 1.4.3 #364

Merged

Conversation

lasofivec commented Jan 6, 2020

Uh oh!

Didou09 left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Jan 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lasofivec commented Jan 7, 2020

Uh oh!

Didou09 left a comment

Choose a reason for hiding this comment

Uh oh!

Didou09 Jan 7, 2020

Choose a reason for hiding this comment

Uh oh!

lasofivec Jan 7, 2020

Choose a reason for hiding this comment

Uh oh!

Didou09 Jan 7, 2020

Choose a reason for hiding this comment

Uh oh!

lasofivec Jan 7, 2020

Choose a reason for hiding this comment

Uh oh!

Didou09 Jan 7, 2020

Choose a reason for hiding this comment

Uh oh!

lasofivec Jan 7, 2020

Choose a reason for hiding this comment

Uh oh!

lasofivec commented Jan 7, 2020

Uh oh!

lasofivec commented Jan 7, 2020

Uh oh!

Didou09 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lasofivec commented Jan 7, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-io commented Jan 7, 2020 •

edited

Loading

Didou09 left a comment •

edited

Loading