Scattering-diffuse-mni #18
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
First version of scattering_diffuse. |
ahms5
reviewed
Jul 26, 2023
imkar/scattering/diffuse.py
Outdated
|
|
||
| Parameters | ||
| ---------- | ||
| reverb_time : pf.FrequencyData |
There was a problem hiding this comment.
Suggested change
| reverb_time : pf.FrequencyData | |
| reverberation_time : pf.FrequencyData |
imkar/scattering/diffuse.py
Outdated
|
|
||
| sample_area : float | ||
| Surface area of the measurement sample in square meter. Standard value | ||
| according to the test sample at IHTA. |
There was a problem hiding this comment.
Suggested change
| according to the test sample at IHTA. | |
| according to the test sample. |
| measurement time. | ||
|
|
||
| calculation_method : 'ISO','Eyring' or 'Sabine' | ||
| Defines the method by which alpha is calculated. |
There was a problem hiding this comment.
``'ISO'``
... baseplate_area and sample_area need to be equal. (add test)
``'Eyring'``
imkar/scattering/diffuse.py
Outdated
| Frequency object containing the reverbaration times calculated during | ||
| the four different measurements. If the measurement is performed at | ||
| multiple positions, the mean value for every condition of the | ||
| measurements is taken. Its cshape has to be (4,). |
There was a problem hiding this comment.
Suggested change
| measurements is taken. Its cshape has to be (4,). | |
| measurements is taken. Its cshape has to be (..., 4) |
| import pytest | ||
| import numpy.testing as npt | ||
|
|
||
|
|
There was a problem hiding this comment.
add test for multidimensional inputs
np.vstack(([T60_1, T60_2, T60_3, T60_4], [T60_1, T60_2, T60_3, T60_4]))
imkar/scattering/diffuse.py
Outdated
| if (np.any(np.asarray(speed_of_sound, dtype=float) <= 0)): | ||
| raise ValueError("speed_of_sound has to be a value bigger than zero") | ||
|
|
||
| if type(air_attenuation) == pf.classes.audio.FrequencyData: |
There was a problem hiding this comment.
if isinstance(air_attenuation, pf.FrequencyData):
imkar/scattering/diffuse.py
Outdated
| raise ValueError("air_attenuation has to be a value bigger or equal \ | ||
| to zero") | ||
|
|
||
| if (calculation_method != "ISO" and calculation_method != "Eyring" and |
There was a problem hiding this comment.
Suggested change
| if (calculation_method != "ISO" and calculation_method != "Eyring" and | |
| if calculation_method in ['ISO', 'Eyring', 'Sabine']: |
MaxNilo
commented
Jan 23, 2024
ahms5
reviewed
Feb 20, 2024
| Parameters | ||
| ---------- | ||
| reverberation_time : pf.FrequencyData | ||
| Frequency object containing the reverbaration times calculated during |
There was a problem hiding this comment.
Suggested change
| Frequency object containing the reverbaration times calculated during | |
| Frequency object containing the reverberation times calculated during |
|
|
||
| scale_factor : float | ||
| Factor that corrects the frequencies when using a smaller scale | ||
| reverbaration room than defined in the ISO standard. Standard value is |
There was a problem hiding this comment.
Suggested change
| reverbaration room than defined in the ISO standard. Standard value is | |
| reverberation room than defined in the ISO standard. Standard value is |
| measurement condition calculated by temperature and air humidity during | ||
| measurement time. Its cshape has to be (..., 4). | ||
|
|
||
| calculation_method : 'ISO','Eyring' or 'Sabine' |
There was a problem hiding this comment.
Suggested change
| calculation_method : 'ISO','Eyring' or 'Sabine' | |
| calculation_method : string |
describe each option as it is done for distance_measure in find_nearest
https://pyfar.readthedocs.io/en/stable/classes/pyfar.coordinates.html#pyfar.classes.coordinates.Coordinates.find_nearest
| raise ValueError("For calculation method 'ISO' the baseplate_area and \ | ||
| sample_area need to be equal.") | ||
|
|
||
| if not isinstance(scale_factor, numbers.Real) or scale_factor <= 0: |
There was a problem hiding this comment.
Suggested change
| if not isinstance(scale_factor, numbers.Real) or scale_factor <= 0: | |
| if not isinstance(scale_factor, (float, int)) or scale_factor <= 0: |
in this way we dont need numbers package anymore
| if not isinstance(scale_factor, numbers.Real) or scale_factor <= 0: | ||
| raise TypeError("scale_factor has to be a real number >0") | ||
|
|
||
| if not isinstance(sample_area, numbers.Real) or sample_area <= 0: |
| if not isinstance(surface_room, numbers.Real) or surface_room <= 0: | ||
| raise TypeError("surface_room has to be a real number >0") | ||
|
|
||
| if not isinstance(volume_room, numbers.Real) or volume_room <= 0: |
| condition calculated by the temperature and air humidity during | ||
| measurement time. | ||
|
|
||
| air_attenuation : array, pf.FrequencyData |
There was a problem hiding this comment.
don't allow array, it's frequency data, so force pf.FrequencyData
Suggested change
| air_attenuation : array, pf.FrequencyData | |
| air_attenuation : pf.FrequencyData |
| raise ValueError("speed_of_sound has to be a value bigger than zero") | ||
|
|
||
| if isinstance(air_attenuation, pf.FrequencyData): | ||
| air_attenuation = np.asarray(air_attenuation.freq, dtype=float) |
There was a problem hiding this comment.
dont convert to array, use pf.FrequencyData
|
|
||
| if (np.any(np.asarray(speed_of_sound, dtype=float) <= 0)): | ||
| raise ValueError("speed_of_sound has to be a value bigger than zero") | ||
|
|
There was a problem hiding this comment.
add a check if frequencies of air_attenuationand reverberation_timesare same.
| (alpha[:, 2]-alpha[:, 0]).clip(min=0) | ||
|
|
||
| # random-incidence scattering coefficient | ||
| scatterring_coefficient = (1-(1-a_spec)/(1-a_s)).clip(min=0) |
There was a problem hiding this comment.
Suggested change
| scatterring_coefficient = (1-(1-a_spec)/(1-a_s)).clip(min=0) | |
| scattering_coefficient = (1-(1-a_spec)/(1-a_s)).clip(min=0) |
|
replaced by #23 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which issue(s) are closed by this pull request?
Closes #
Changes proposed in this pull request:
Labels
Label your issue to make it easier for us to assign and track:
Use one of these labels: