pyroomacoustics icon indicating copy to clipboard operation
pyroomacoustics copied to clipboard

When I try room.compute_rir() again,is it intended that the ISM or Ray-Tracing will not be re-modeled?

Open AlanLiudx opened this issue 2 years ago • 5 comments

Hello faku @fakufaku and thanks for your continuous contribution! I'm new here to learn multi-channel acoustic scene simulation, and I found something weird when trying to compute rir:

def compute_rir(self):
        """
        Compute the room impulse response between every source and microphone.
        """

        if self.simulator_state["ism_needed"] and not self.simulator_state["ism_done"]:
            self.image_source_model()

        if self.simulator_state["rt_needed"] and not self.simulator_state["rt_done"]:
            self.ray_tracing()

        self.rir = []

In the code block above, I see if I change the parameters in the existing ISM or RT model such as max_order(for instance, I first try max_order=0 to generate anechoic speech and then set different sets of RT60 and generate reverberated speech), the ISM or RT will not be re-modeled because self.simulator_state["ism_done"] has been set True in the end of the method of self.image_source_model() or self.ray_tracing(). I wonder if that is a feature or a bug. I think if I want to change nothing in the room but the reverberation time I need, there's no means for me to create another room for another set of speech. Of course I can manually reset self.simulator_state["ism_done"], but seemingly in this way I break the encapsulation of the Room class. Could you please give me an explanation? I would be grateful for that.

AlanLiudx avatar Jun 15 '22 11:06 AlanLiudx

Hi @AlanLiudx , yes this is more a feature than a bug. There is a lot of states in the internals, and because you can call intermediate functions (such as compute_rir, like you are trying to do), it is difficult to know when re-initialization is important. Right now reseting things in self.simulator_state is the way to go, although not ideal, as you point out. I have been thinking adding a reset method to the room object to take care of that.

fakufaku avatar Jun 15 '22 13:06 fakufaku

Thank you for your explanation! But actually I read the source code after I found my test code hadn't acted as I 'd expected, and what if a beginner simulates the room impulse and finds nothing changes, getting confused? Perhaps some hints should be added in the examples or documentations. Thank you!

AlanLiudx avatar Jun 16 '22 02:06 AlanLiudx

You are totally correct, this is far from an ideal situation... 😰

fakufaku avatar Jun 16 '22 02:06 fakufaku

I tried to implement a reset_simulator_state method outside the class (outside the source code room.py) without doc. Perhaps in this way a reset method can be tried to be implemented into the source?

from __future__ import division, print_function
import pyroomacoustics
from pyroomacoustics import Room
import numpy as np


import math
import warnings

import numpy as np
import scipy.spatial as spatial

def reset_simulator_state(self):
    self.simulator_state["ism_needed"]=True
    self.simulator_state["rt_needed"]=False
    self.simulator_state["ism_done"]=False
    self.simulator_state["rt_done"]=False

Room.reset_simulator_state=reset_simulator_state

AlanLiudx avatar Jun 17 '22 05:06 AlanLiudx

Thanks for sharing! I think the values of ism_needed and rt_needed depend on the options provided when constructing the object. Another solution would be to not have intermediate methods (like compute_rir) and make everything be done by simulate. Then, at then end of the simulate methods, the values of ism_done and rt_done can be re-initialized.

I think I had originally done that because in some cases we want to keep the same RIR, but change only the source signals. In this case we don't want to re-run compute_rir everytime.

fakufaku avatar Jun 17 '22 06:06 fakufaku