JeschkeLab / DeerLab-Matlab

Data analysis and method development toolbox for dipolar EPR spectroscopy
MIT License
4 stars 2 forks source link

New options system is a performance bottleneck #111

Closed luisfabib closed 4 years ago

luisfabib commented 4 years ago

After the recent re-design of fitsignal (47cb48a), while running the corresponding tests I realized that the performance had dropped dramatically. Here is an example of the performance drop found for a simple test such as fitsignal_4pdeer

Before
fitsignal_4pdeer                         pass     1.42e-01  1.426 sec 
Now
fitsignal_4pdeer                         pass     1.51e-01  35.885 sec 

Profiling the test run clearly shows that the bottleneck is the parseoptions function, especially the call to dbstack, which and load functions. This is unacceptable as the parseoptions should at most run for 1s (if even). The reason why this was not detected while implemening this was that the runtime is not that dramatic for single-call functions. However, in the case of fitsignal, it internally calls dipolarkernel>dipolarbackground inside the model function which is evaluated by the solver. Therefore, parseoptions is called over 20k times leading to the performance drop.

Picture1

luisfabib commented 4 years ago

I tried making the cache, stack, and DeerLab_path persistent variables to avoid their repeated evaluation. With stack it is not possible as the function is dependent on the internal state of MATLAB and cannot be memoized or cached. While this changes considerably reduce the self time of parseoptions it still remains the bottleneck of fitsignal.

Picture2

We might need to drop this options design, and return to a less sophisticated but faster one.

stestoll commented 4 years ago

I agree. It's both too complicated and too slow. Easiest if each function just looks for the name-value pairs it needs and then ignores all others.

luisfabib commented 4 years ago

Closed with d53a2be