Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speedup sofa #45

Closed
wants to merge 6 commits into from
Closed

Speedup sofa #45

wants to merge 6 commits into from

Conversation

hagenw
Copy link
Member

@hagenw hagenw commented Oct 13, 2015

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
Copy link
Member Author

hagenw commented Oct 14, 2015

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
Copy link
Member

fietew commented Oct 14, 2015

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:

  • iterate over the head orientations and the secondary source position in order to get the indices of the impulse responses, which are needed from the SOFA file. This can be done by evaluating the header of the SOFA file, i.e. using nearest_neighbor, etc. (as you have done it in the current implementation)
  • the indices are likely to be non-unique, as some head-orientations-ssd-position pairs might need the same impulse responses. However, functions like unique (http://de.mathworks.com/help/matlab/ref/unique.html) help us to remove those duplicates.
  • sorting the indices might also help to identify segments of connected indices, i.e. groups of directly subsequent indices, e.g. [2,3,4,5] [7,8,9] [11,12]. As I experienced, that loading the impulse responses index by index is quite slow in SOFA, one could to something like https://github.com/TWOEARS/binaural-simulator/blob/master/src/%2Bsimulator/DirectionalIR.m#L189-L202 .
  • after loading the impulse responses, they have to be processed according to the head-orientations-ssd-position pairs they are need for, i.e. distance adjustment, angle interpolation, adding up all loudspeakers.

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
Copy link
Member Author

hagenw commented Oct 14, 2015

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 hagenw added the wontfix label Feb 29, 2016
@hagenw hagenw mentioned this pull request Feb 29, 2016
@hagenw
Copy link
Member Author

hagenw commented Feb 29, 2016

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

@hagenw hagenw closed this Feb 29, 2016
@hagenw hagenw deleted the speedup_sofa branch February 29, 2016 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants