sfstoolbox / sfs-matlab

SFS Toolbox for Matlab/Octave
https://sfs-matlab.readthedocs.io
MIT License
98 stars 39 forks source link

Speedup sofa #45

Closed hagenw closed 8 years ago

hagenw commented 8 years ago

This version includes some time optimizations for using SOFA together with ir_generic(). It needs some more testing to see, if all the SOFA stuff is ready now.

hagenw commented 8 years ago

I'm still not sure if the changes in the code are worth for integration.

The speedup comes to play only for very huge numbers of loudspeakers (>10000) or if you use the ssr_brs_wfs() function with a normal to large number of loudspeakers. For example, for 800 loudspeakers, the execution time with the master branch was ~1200 s and with the changes applied here ~800 s. On the downside the structure of the code is not so nice in speedup_sofa, we need an additional get_ir_loop() function and have some additional lines in ir_generic() that are not nice.

fietew commented 8 years ago

As far as I understood the current implementation, the SOFA file is accessed for each head orientation and for each loudspeaker position separately. This means, that an impulse response (having e.g. the index "42" ) is loaded from the hard disk drive into the RAM several times. If we really want to speed up things, then this architecture has to be reassessed. An alternative workflow could be:

The major drawback of this approach is, that there might be a huge number of impulse responses which are loaded into the RAM at once. One could define a maximum number of irs loaded in one step and then iterate as long as there are impulse responses left to be processed. However, one would have ensure, that there is always at least one head-orientations-ssd-position pair which can be processed with the current impulse responses in the RAM.

hagenw commented 8 years ago

I agree, there is more we could do and your points sound meaningful, but also not too easy to implement. I would propose that we leave the current (as it is in master) SOFA implementation for the next Toolbox release. When we have time, we could see how to proceed with this pull request.

hagenw commented 8 years ago

The implementation presented here, will not be used. For further considerations along the lines of @fietew proposal have a look at #67.