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

Potential bug when generating RT60 using v0.7.5 #358

Closed
michaeljconnolly opened this issue Jul 17, 2024 · 6 comments
Closed

Potential bug when generating RT60 using v0.7.5 #358

michaeljconnolly opened this issue Jul 17, 2024 · 6 comments
Assignees

Comments

@michaeljconnolly
Copy link

Hi,

Running the pyroomacoustics_demo notebook, I've noticed that the RT60 generated using version 0.7.5 is significantly different than the RT60 generated using versions 0.7.3 and 0.7.4.

I tested further with my own scripts, that are similar to the demo notebook (both attached here: scripts.zip). One uses ray tracing (raytracing.py), and the other uses ISM (ism.py).

For raytracing, the RT60 is ~40 ms for v0.7.5, but ~280 ms for both v0.7.3 and v0.7.4.

For ISM, the RT60 is 7 ms for v0.7.5, but 31 ms for both v0.7.3 and v0.7.4.

I have reproduced these results using docker containers, built with the provided Dockerfile (changing pyroomacoustics version only).

I wonder if this is a bug, or if I'm missing something?

Thanks for your help.

@fakufaku
Copy link
Collaborator

@michaeljconnolly thanks for the heads up!
Actually, I have updated the code for the RT60 to be more robust.
The new code will ignore outlier samples at the tail of the energy curve to try to get a better fit.
Have you tried to check which results makes more sense wrt to the actual RIR?
If you provide plot=True as an argument to measure_rt60, it should also show a plot that should make it clear if there is a problem or not.

@fakufaku fakufaku self-assigned this Jul 18, 2024
@michaeljconnolly
Copy link
Author

You're welcome, and thanks for your response!

I have plotted the RIR for one microphone and the RT60 for 0.7.3, 0.7.4 and 0.7.5. These examples closely follow the demo notebook (ray tracing with 1000 rays, max_order 3).

It looks like, the RIR tails off at ~150ms. It is interesting that for the 0.7.5 RT60 plot, the energy does not fall to -60dB.

Images below:

0.7.3
0 7 3

0.7.4
0 7 4

0.7.5
0 7 5

Thanks again for your feedback.

@fakufaku
Copy link
Collaborator

Thanks! It looks like the default value of the energy_thresh parameter is too low. Could you please try to set it to 0.99 or 0.999?

@michaeljconnolly
Copy link
Author

No problem. I have added some plots for v0.7.5, setting energy_thresh to 0.99, 0.999 and 1.0. The RT60 lines up with 0.7.3 and 0.7.4, when set to 1.0. With energy_thresh set to 0.99 and 0.999, the energy does not fall to -60dB.

Images below:

energy_thresh 0.99
thres_0 99

energy_thresh 0.999
thres_0 999

energy_thresh 1.0
thres_1 0

@fakufaku
Copy link
Collaborator

fakufaku commented Aug 5, 2024

It looks like the energy_thres parameter is more difficult to use than I anticipated.
For now I will change the default to 1.0 to match the previous behavior of the function.

fakufaku added a commit that referenced this issue Aug 5, 2024
@michaeljconnolly
Copy link
Author

Perfect, thanks for your help with this!

@fakufaku fakufaku closed this as completed Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants