sympy icon indicating copy to clipboard operation
sympy copied to clipboard

Improve sympy import time by eliminating redundant ordering of Dispatcher

Open eendebakpt opened this issue 3 years ago • 10 comments

Brief description of what is fixed or changed

During the sympy import several Dispatchers are created and methods are registered. On each registration the full set of methods is ordered, which is costly.

This PR reduces the sorting by

  1. Stopping the sorting before importing subpackages
  2. After importing subpackges (which includes the registration of methods to dispatchers), sort all dispatchers

Existing functionality of the Dispatcher is used.

The import time is reduced from 269 to 258 ms.

Output of bin/test_import

Master:

eendebakpt@woelmuis:/mnt/data/sympy$ bin/test_import 
Note: the first run (warm up) was not included in the average + std dev
All runs (including warm up):
[0.2750202629999876, 0.27497193600015635, 0.2702798200000416, 0.26889375400014615, 0.2687653349998982, 0.2716369390000182, 0.2794790110001486, 0.26655530899984115, 0.2691565279999395, 0.2684384439999121, 0.27359299299996565, 0.26817051800003355, 0.26997147999986737, 0.26848315500001263, 0.2695972559999973, 0.2689053330000206, 0.266499002999808, 0.27336413400007586, 0.2695815440001752, 0.2726911500001279, 0.2669147869999051, 0.26745493099997475, 0.27697483599990846, 0.26868902599994726, 0.2712855639999816, 0.26795200600008684, 0.268541746999972, 0.26775163699994664, 0.27113610199990035, 0.2674677069999234, 0.26795610500016664, 0.26821584900017115, 0.26595949700003985, 0.2713385689999086, 0.2666728149999926, 0.2681342289999975, 0.2687668170001416, 0.2691814659999636, 0.26562634600009005, 0.26544889799993143, 0.2664543210000829, 0.2697006410000995, 0.2687029410001287, 0.26908488100002614, 0.2688644029999523, 0.2712997159999304, 0.27255410699990534, 0.2704326430000492, 0.270944481000015, 0.2717818040000566, 0.2686064910001278]
Number of tests: 50
The speed of "import sympy" is: 0.269579 +- 0.002746
eendebakpt@woelmuis:/mnt/data/sympy$ bin/test_import 
Note: the first run (warm up) was not included in the average + std dev
All runs (including warm up):
[0.27012507099993854, 0.2690900089999104, 0.2714201490000505, 0.266401506999955, 0.269292661999998, 0.2699106360000769, 0.2750912349999908, 0.2690173099999811, 0.2662025030001587, 0.27493229600008817, 0.2732122560000789, 0.26986700499992367, 0.2673109410000052, 0.2685452460000306, 0.2705744620000132, 0.27552572399986275, 0.26664292999998906, 0.2703416899998956, 0.27229860700003883, 0.2681302329999653, 0.26659810000001016, 0.2696212359999208, 0.2680760770001598, 0.2669595599998047, 0.2689367319999292, 0.2691656189999776, 0.2738177270000506, 0.270048332999977, 0.27325413200014737, 0.26922133700009, 0.2687775770000371, 0.2711154799999349, 0.2729384140000093, 0.2654830999999831, 0.270534656999871, 0.27074404199993296, 0.2697775599999659, 0.2736168829999315, 0.26733127000011336, 0.2783130220000203, 0.2690745869999773, 0.2671201389998714, 0.26748783299990464, 0.2705764470001668, 0.26714791799986415, 0.26750925000010284, 0.2674039340001855, 0.2647815189998255, 0.2660385930000757, 0.2756670290000329, 0.26935999700003777]
Number of tests: 50
The speed of "import sympy" is: 0.269806 +- 0.002957

PR

eendebakpt@woelmuis:~/sympy$ bin/test_import 
Note: the first run (warm up) was not included in the average + std dev
All runs (including warm up):
[0.25928017599994746, 0.25534695700002885, 0.25327333200016255, 0.2605424830001084, 0.25428364799995506, 0.25779407999993964, 0.25567224200017336, 0.255802980999988, 0.2575704589999077, 0.2656662080000842, 0.25502123999990545, 0.25856492699995215, 0.2610679979998167, 0.256039249999958, 0.2549249769999733, 0.26770503499983533, 0.2585111280000092, 0.2534973199999513, 0.2574933930000043, 0.2688835730000392, 0.25709193600005165, 0.2582924620000995, 0.2551701239999602, 0.2579445010001109, 0.2537198909999461, 0.258160615999941, 0.257400453000173, 0.2575714600000083, 0.25719076500013216, 0.25602168299997174, 0.2612546560001192, 0.2569049520000135, 0.25658759199995984, 0.25826334699991094, 0.25390822100007426, 0.25522967400002017, 0.25978250799994385, 0.2568119569998544, 0.2612774450001325, 0.2602062579999256, 0.26707800600001974, 0.25654055499990136, 0.25464717400018344, 0.2567932940000901, 0.2616354109998156, 0.25789041799998813, 0.26002433899998323, 0.25881536300016705, 0.25908980100007284, 0.2625190040000689, 0.2601776740000332]
Number of tests: 50
The speed of "import sympy" is: 0.258233 +- 0.003504
eendebakpt@woelmuis:~/sympy$ bin/test_import 
Note: the first run (warm up) was not included in the average + std dev
All runs (including warm up):
[0.2627209590000348, 0.2606937199998356, 0.25977801800013367, 0.2668773880000117, 0.25525705599989124, 0.2595361710000361, 0.2584197160001622, 0.25180167500002426, 0.25751827199997024, 0.25490321699999186, 0.2551755930001036, 0.2596296629999415, 0.2635841750000054, 0.2573761200001172, 0.2553685039999891, 0.2559703060001084, 0.25835445799998524, 0.2524185590000343, 0.2570603270000902, 0.25519752699983655, 0.2634718380002141, 0.2549482720000924, 0.25889327100003356, 0.25680885099995976, 0.2593443959999604, 0.2589607429999887, 0.252508078000119, 0.25633841100011523, 0.26284038900007545, 0.25504915600004097, 0.2614197939999485, 0.25328517200000533, 0.25799999299988485, 0.25643479199993635, 0.2616250169999148, 0.2559688650001135, 0.2564684930000567, 0.26460269800008973, 0.2579310290000194, 0.2577397129998644, 0.2574612999999317, 0.2559228080001503, 0.2592914419999488, 0.2607275979999031, 0.2515977670000211, 0.256944552999812, 0.258465249999972, 0.2566703930001495, 0.2568872580000061, 0.2558636070000375, 0.25956973699999253]
Number of tests: 50
The speed of "import sympy" is: 0.257739 +- 0.003201

Other comments

The registration inside matmul and matadd is a bit special. The methods registered there are ambiguous, but this is accepted by passing a special ambiguity_register_error_ignore_dup as the on_ambiguity argument. For this reason we temporary enable the ordering when registering inside these modules.

Release Notes

NO ENTRY

eendebakpt avatar Jun 12 '22 19:06 eendebakpt

:white_check_mark:

Hi, I am the SymPy bot (v167). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

  • No release notes entry will be added for this pull request.
Click here to see the pull request description that was parsed.
<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### Brief description of what is fixed or changed

During the sympy import several Dispatchers are created and methods are registered. On each registration the full set of methods is ordered, which is costly. 

This PR reduces the sorting by

1) Stopping the sorting before importing subpackages
2) After importing subpackges (which includes the registration of methods to dispatchers), sort all dispatchers

Existing functionality of the `Dispatcher` is used.

The import time is reduced from 269 to 258 ms.

<details>
<summary>Output of bin/test_import</summary>

Master: 
```
eendebakpt@woelmuis:/mnt/data/sympy$ bin/test_import 
Note: the first run (warm up) was not included in the average + std dev
All runs (including warm up):
[0.2750202629999876, 0.27497193600015635, 0.2702798200000416, 0.26889375400014615, 0.2687653349998982, 0.2716369390000182, 0.2794790110001486, 0.26655530899984115, 0.2691565279999395, 0.2684384439999121, 0.27359299299996565, 0.26817051800003355, 0.26997147999986737, 0.26848315500001263, 0.2695972559999973, 0.2689053330000206, 0.266499002999808, 0.27336413400007586, 0.2695815440001752, 0.2726911500001279, 0.2669147869999051, 0.26745493099997475, 0.27697483599990846, 0.26868902599994726, 0.2712855639999816, 0.26795200600008684, 0.268541746999972, 0.26775163699994664, 0.27113610199990035, 0.2674677069999234, 0.26795610500016664, 0.26821584900017115, 0.26595949700003985, 0.2713385689999086, 0.2666728149999926, 0.2681342289999975, 0.2687668170001416, 0.2691814659999636, 0.26562634600009005, 0.26544889799993143, 0.2664543210000829, 0.2697006410000995, 0.2687029410001287, 0.26908488100002614, 0.2688644029999523, 0.2712997159999304, 0.27255410699990534, 0.2704326430000492, 0.270944481000015, 0.2717818040000566, 0.2686064910001278]
Number of tests: 50
The speed of "import sympy" is: 0.269579 +- 0.002746
eendebakpt@woelmuis:/mnt/data/sympy$ bin/test_import 
Note: the first run (warm up) was not included in the average + std dev
All runs (including warm up):
[0.27012507099993854, 0.2690900089999104, 0.2714201490000505, 0.266401506999955, 0.269292661999998, 0.2699106360000769, 0.2750912349999908, 0.2690173099999811, 0.2662025030001587, 0.27493229600008817, 0.2732122560000789, 0.26986700499992367, 0.2673109410000052, 0.2685452460000306, 0.2705744620000132, 0.27552572399986275, 0.26664292999998906, 0.2703416899998956, 0.27229860700003883, 0.2681302329999653, 0.26659810000001016, 0.2696212359999208, 0.2680760770001598, 0.2669595599998047, 0.2689367319999292, 0.2691656189999776, 0.2738177270000506, 0.270048332999977, 0.27325413200014737, 0.26922133700009, 0.2687775770000371, 0.2711154799999349, 0.2729384140000093, 0.2654830999999831, 0.270534656999871, 0.27074404199993296, 0.2697775599999659, 0.2736168829999315, 0.26733127000011336, 0.2783130220000203, 0.2690745869999773, 0.2671201389998714, 0.26748783299990464, 0.2705764470001668, 0.26714791799986415, 0.26750925000010284, 0.2674039340001855, 0.2647815189998255, 0.2660385930000757, 0.2756670290000329, 0.26935999700003777]
Number of tests: 50
The speed of "import sympy" is: 0.269806 +- 0.002957
```

PR
```
eendebakpt@woelmuis:~/sympy$ bin/test_import 
Note: the first run (warm up) was not included in the average + std dev
All runs (including warm up):
[0.25928017599994746, 0.25534695700002885, 0.25327333200016255, 0.2605424830001084, 0.25428364799995506, 0.25779407999993964, 0.25567224200017336, 0.255802980999988, 0.2575704589999077, 0.2656662080000842, 0.25502123999990545, 0.25856492699995215, 0.2610679979998167, 0.256039249999958, 0.2549249769999733, 0.26770503499983533, 0.2585111280000092, 0.2534973199999513, 0.2574933930000043, 0.2688835730000392, 0.25709193600005165, 0.2582924620000995, 0.2551701239999602, 0.2579445010001109, 0.2537198909999461, 0.258160615999941, 0.257400453000173, 0.2575714600000083, 0.25719076500013216, 0.25602168299997174, 0.2612546560001192, 0.2569049520000135, 0.25658759199995984, 0.25826334699991094, 0.25390822100007426, 0.25522967400002017, 0.25978250799994385, 0.2568119569998544, 0.2612774450001325, 0.2602062579999256, 0.26707800600001974, 0.25654055499990136, 0.25464717400018344, 0.2567932940000901, 0.2616354109998156, 0.25789041799998813, 0.26002433899998323, 0.25881536300016705, 0.25908980100007284, 0.2625190040000689, 0.2601776740000332]
Number of tests: 50
The speed of "import sympy" is: 0.258233 +- 0.003504
eendebakpt@woelmuis:~/sympy$ bin/test_import 
Note: the first run (warm up) was not included in the average + std dev
All runs (including warm up):
[0.2627209590000348, 0.2606937199998356, 0.25977801800013367, 0.2668773880000117, 0.25525705599989124, 0.2595361710000361, 0.2584197160001622, 0.25180167500002426, 0.25751827199997024, 0.25490321699999186, 0.2551755930001036, 0.2596296629999415, 0.2635841750000054, 0.2573761200001172, 0.2553685039999891, 0.2559703060001084, 0.25835445799998524, 0.2524185590000343, 0.2570603270000902, 0.25519752699983655, 0.2634718380002141, 0.2549482720000924, 0.25889327100003356, 0.25680885099995976, 0.2593443959999604, 0.2589607429999887, 0.252508078000119, 0.25633841100011523, 0.26284038900007545, 0.25504915600004097, 0.2614197939999485, 0.25328517200000533, 0.25799999299988485, 0.25643479199993635, 0.2616250169999148, 0.2559688650001135, 0.2564684930000567, 0.26460269800008973, 0.2579310290000194, 0.2577397129998644, 0.2574612999999317, 0.2559228080001503, 0.2592914419999488, 0.2607275979999031, 0.2515977670000211, 0.256944552999812, 0.258465249999972, 0.2566703930001495, 0.2568872580000061, 0.2558636070000375, 0.25956973699999253]
Number of tests: 50
The speed of "import sympy" is: 0.257739 +- 0.003201
```
</details>

#### Other comments

The registration inside `matmul` and `matadd` is a bit special. The methods registered there are ambiguous, but this is accepted by passing a special `ambiguity_register_error_ignore_dup` as the `on_ambiguity` argument. 
For this reason we temporary enable the ordering when registering inside these modules.

#### Release Notes

<!-- Write the release notes for this release below between the BEGIN and END
statements. The basic format is a bulleted list with the name of the subpackage
and the release note for this PR. For example:

* solvers
  * Added a new solver for logarithmic equations.

* functions
  * Fixed a bug with log of integers.

or if no release note(s) should be included use:

NO ENTRY

See https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more
information on how to write release notes. The bot will check your release
notes automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
NO ENTRY
<!-- END RELEASE NOTES -->

sympy-bot avatar Jun 12 '22 19:06 sympy-bot

Benchmark results from GitHub Actions

Lower numbers are good, higher numbers are bad. A ratio less than 1 means a speed up and greater than 1 means a slowdown. Green lines beginning with + are slowdowns (the PR is slower then master or master is slower than the previous release). Red lines beginning with - are speedups.

Significantly changed benchmark results (PR vs master)


Significantly changed benchmark results (master vs previous release)


Full benchmark results can be found as artifacts in GitHub Actions (click on checks at the top of the PR).

github-actions[bot] avatar Jun 12 '22 20:06 github-actions[bot]

Would this work if SymPy was using the PyPI multipledispatch module? I'd really like to get rid of SymPy's multipledispatch module and use that instead.

oscarbenjamin avatar Jun 14 '22 22:06 oscarbenjamin

Would this work if SymPy was using the PyPI multipledispatch module? I'd really like to get rid of SymPy's multipledispatch module and use that instead.

The pypi multipledispatch and sympy multipledispatch are obviously related. But there are differences, compare for example https://github.com/mrocklin/multipledispatch/blob/master/multipledispatch/conflict.py#L9 with https://github.com/sympy/sympy/blob/master/sympy/multipledispatch/conflict.py#L7. Replacing the dispatchers with the pypi dispatcher is not something I will pick up.

eendebakpt avatar Jun 15 '22 07:06 eendebakpt

Replacing the dispatchers with the pypi dispatcher is not something I will pick up.

Fair enough but my point is that I don't want to introduce any changes that would be incompatible with the other multipledispatch module.

oscarbenjamin avatar Jun 15 '22 17:06 oscarbenjamin

It doesn't look like this changes anything in multipledispatch, so I don't see that being an issue.

asmeurer avatar Jun 15 '22 20:06 asmeurer

Although these are apparently deprecated upstream https://github.com/mrocklin/multipledispatch/blob/master/multipledispatch/dispatcher.py#L30-L47.

asmeurer avatar Jun 15 '22 20:06 asmeurer

I guess upstream made the lazy behavior the default. I wonder if it is faster than this.

This change looks good for now, though.

asmeurer avatar Jun 15 '22 20:06 asmeurer

Personally, I'm no fan of having pairs of calls at module level that shares and mutates global state. I would at least like to see such pairs wrapped in a context manager.

bjodah avatar Jun 16 '22 11:06 bjodah

I opened #23830 to consider the possibility of using the pypi multipledispatch module.

oscarbenjamin avatar Jul 25 '22 16:07 oscarbenjamin