Sabine reverberation time #103
Conversation
TimSchellekensLAV
left a comment
TimSchellekensLAV
left a comment
There was a problem hiding this comment.
Thank you for your contribution.
Only missing some units.
pyrato/parametric.py
Outdated
| alphas : ndarray, double | ||
| Absorption coefficients corresponding to each surface | ||
| volume : double | ||
| Room volume |
There was a problem hiding this comment.
We agree and provide changes. Thank You!
pyrato/parametric.py
Outdated
| Returns | ||
| ------- | ||
| reverberation_time_sabine : double | ||
| The value of calculated reverberation time |
There was a problem hiding this comment.
We agree and wil provide changes. Thank You!
ahms5
added
the
ASSA 2025
mberz
left a comment
mberz
left a comment
There was a problem hiding this comment.
Thanks for your contribution.
I have realized that there is no check if the length of the input arrays match. The line calculating the absorption area however will crash if users don't account for it.
I think this needs to be addressed.
pyrato/parametric.py
Outdated
| alphas = np.asarray(alphas) | ||
| surfaces = np.asarray(surfaces) |
There was a problem hiding this comment.
Here it may be beneficial to check if the size of both arrays are equal to catch potential user input errors.
| alphas = np.asarray(alphas) | |
| surfaces = np.asarray(surfaces) | |
| alphas = np.asarray(alphas) | |
| surfaces = np.asarray(surfaces) | |
| if surfaces.shape != alphas.shape: | |
| raise ValueError("The shapes of alphas and surfaces do not match.") |
This information also would be good to have in the docstring..
Co-authored-by: Marco Berzborn <mberz@users.noreply.github.com>
mberz
left a comment
mberz
left a comment
There was a problem hiding this comment.
Thanks for the update. Looking quite good already.
ahms5
left a comment
ahms5
left a comment
There was a problem hiding this comment.
Thank you for your contribution. Look very nice, I just have some suggestions on docs.
pyrato/parametric.py
Outdated
| alphas = np.asarray(alphas) | ||
|
|
||
| if alphas.shape != surfaces.shape: | ||
| raise ValueError("Size of alphas and surfaces ndarray sizes must match.") |
There was a problem hiding this comment.
Maybe sth like shapes of alphas and surfaces must match.
pyrato/parametric.py
Outdated
| if np.any(alphas < 0) or np.any(alphas > 1): | ||
| raise ValueError(f"Absorption coefficient values must be in range [0, 1]. Got {alphas}.") | ||
| if np.any(surfaces < 0): | ||
| raise ValueError(f"Surface areas cannot be negative. Got {surfaces}.") | ||
| if volume < 0: | ||
| raise ValueError(f"Volume cannot be negative. Got {volume}.") |
…s/pyrato into sabine_reverberation_time
Co-authored-by: Anne Heimes <64446926+ahms5@users.noreply.github.com>
4 participants
Which issue(s) are closed by this pull request?
Closes #84
Changes proposed in this pull request: