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

New scattering freefield implementation #12

Open
wants to merge 33 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
a283387
impl new approach for scattering
ahms5 Feb 24, 2023
bcb4fbc
make diffusion module available
ahms5 Feb 24, 2023
38eefd7
Revert "make diffusion module available"
ahms5 Feb 24, 2023
a88b34f
add formula to doc
ahms5 Mar 10, 2023
5b8a350
fix doc
ahms5 Mar 10, 2023
ac207bc
add paris formula to docs
ahms5 Mar 10, 2023
800b72c
fix docs
ahms5 Mar 10, 2023
2227ef4
Apply suggestions from code review
ahms5 Mar 24, 2023
47802e4
new structure
ahms5 Apr 14, 2023
112cf90
better labeling
ahms5 Apr 14, 2023
0baf823
fix new structure in docs
ahms5 Apr 16, 2023
00f76b1
should not be part of this pr
ahms5 Apr 16, 2023
b29353f
remove stubs as proposed by @sikersten
ahms5 Apr 16, 2023
5c7408e
separate paris formula in utils as discussed
ahms5 Apr 16, 2023
0987a4a
flake8
ahms5 Apr 16, 2023
bb33688
Update imkar/utils/utils.py
ahms5 May 23, 2023
d0544c9
Update imkar/utils/utils.py
ahms5 May 23, 2023
87c48bf
add comments @pingelit
ahms5 May 23, 2023
ada05f8
cosmetics
ahms5 May 23, 2023
df55753
add review @f-brinkmann
ahms5 Jun 12, 2023
4d892eb
try to add reference in doc to pyfar
ahms5 Jun 12, 2023
f993b39
cosmetics + integrate last review comments
ahms5 Jun 12, 2023
02ef6db
cosmetics + integrate last review comments
ahms5 Jun 12, 2023
8d3030e
fix tests
ahms5 Jun 12, 2023
e005c98
fix doc
ahms5 Jun 12, 2023
ac8de6f
Update imkar/scattering/scattering.py
ahms5 Aug 7, 2023
08d0b0f
Update imkar/scattering/scattering.py
ahms5 Aug 7, 2023
ae4140d
Update imkar/utils/utils.py
ahms5 Aug 7, 2023
4f98a5b
apply review @f-brinkmann
ahms5 Aug 7, 2023
599cd70
apply cookiecutter
ahms5 Feb 20, 2024
723c48c
fix flake8
ahms5 Feb 20, 2024
ffaad46
Merge branch 'cookiecutter' into new_scattering_freefield
ahms5 Feb 20, 2024
08504b3
Revert "Merge branch 'cookiecutter' into new_scattering_freefield"
ahms5 Feb 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/modules.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ according to their modules.
.. toctree::
:maxdepth: 1


modules/imkar.scattering
7 changes: 7 additions & 0 deletions docs/modules/imkar.scattering.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
imkar.scattering
================

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A brief general description of the module in a sentence or two might be nice to add.

.. automodule:: imkar.scattering
:members:
:undoc-members:
:show-inheritance:
4 changes: 4 additions & 0 deletions imkar/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,7 @@
__author__ = """The pyfar developers"""
__email__ = 'info@pyfar.org'
__version__ = '0.1.0'


from . import scattering # noqa
from . import utils # noqa
9 changes: 9 additions & 0 deletions imkar/scattering/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from .scattering import (
freefield,
random,
)

__all__ = [
'freefield',
'random',
]
129 changes: 129 additions & 0 deletions imkar/scattering/scattering.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import numpy as np
import pyfar as pf
Comment on lines +1 to +2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See issue - dependencies must be added to setup.py

from imkar import utils


def freefield(sample_pressure, reference_pressure, microphone_weights):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be stupid me and the long while since I had to deal with computing scattering coefficients, but I'm confused with the following:

If I pass 10-channel, 3-frequencies data the function returns single-channel, 3-frequencies data, but the docstring mentions that the scattering is computed for each incidence. Can you make more clear what the angles _S and _R are and how they are encoded in the input and output data?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use None as a default that uses equal weights?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, it does not need to be for each incident angle, because it depends on the input data, what ever is in the dimentions before the last one, gets retured, so I delete the reference for each incident angle.

if we use None, we need the data points instead, and it will get more complex,

Copy link
Member

@f-brinkmann f-brinkmann Jul 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ment None might be a good default to compute equal weights inside the function (avoid passing the data points). But that might not make sense if equal weights are very uncommon for these measurements.

r"""
Calculate the free-field scattering coefficient for each incident direction
using the Mommertz correlation method [1]_:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we go full Zen of Python and start with a one liner without reference? I would also not use incident direction, since this is not passed and seems to be implicitly contained in the input data.

Suggested change
Calculate the free-field scattering coefficient for each incident direction
using the Mommertz correlation method [1]_:
Calculate the direction dependent free-field scattering coefficient.
Uses the Mommertz correlation method [1]_ to calculate the scattering
coefficient separately for each channel of the input data:


.. math::
s(\vartheta_S,\varphi_S) = 1 -
\frac{|\sum \underline{p}_{sample}(\vartheta_R,\varphi_R) \cdot
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For readability I would suggest to use text in the subscripts, e.g.: p_{\text{sample}}

\underline{p}_{reference}^*(\vartheta_R,\varphi_R) \cdot w|^2}
{\sum |\underline{p}_{sample}(\vartheta_R,\varphi_R)|^2 \cdot w
\cdot \sum |\underline{p}_{reference}(\vartheta_R,\varphi_R)|^2
\cdot w }

with the ``sample_pressure``, the ``reference_pressure``, and the
area weights ``weights_microphones``. See
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I would focus more on explaining the formula, e.g., with the sample pressure p_{\text{sample}} and not refer to the variable names here. To me that was clear due to the verbose variable names already.

  • Can we use \sum_w and w(phi_R, theta_R) to make things more clear?

:py:func:`random_incidence` to calculate the random incidence
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Broken reference, should be

Suggested change
:py:func:`random_incidence` to calculate the random incidence
:py:func:`~random` to calculate the random incidence

tilde only displays the function name.

scattering coefficient.

Parameters
----------
sample_pressure : pyfar.FrequencyData
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we think of a way to make this usable with pyfar.Signals? At the moment it seems to be taylored to simulation results in frational octave bands. Could be done by an optional parameter that takes a pyfar filter-bank object. But it might be messy, because in this case FrequencyData objects would not work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should not make it extra complex, its the users responsibilty to convert it to FrequencyData. we could think to allow signals by just using the freq component of it, so without any filtering. then its stay more more open

Reflected sound pressure or directivity of the test sample. Its cshape
need to be (..., #microphones).
reference_pressure : pyfar.FrequencyData
Reflected sound pressure or directivity of the
reference sample. Needs to have the same cshape and frequencies as
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we link to pyfar audio concepts when refering to cshape? Otherwise people less familiar with pyfar might be confused.

`sample_pressure`.
microphone_weights : np.ndarray
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the be an array like and we use np.asarray(weights) or np.atleast1d(weights)?

Array containing the area weights for the microphone positions.
Its shape needs to be (#microphones), so it matches the last dimension
in the cshape of `sample_pressure` and `reference_pressure`.

Returns
-------
scattering_coefficients : pyfar.FrequencyData
The scattering coefficient for each incident direction.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mention the frequency dependency of the output.



References
----------
.. [1] E. Mommertz, „Determination of scattering coefficients from the
reflection directivity of architectural surfaces“, Applied
Acoustics, Bd. 60, Nr. 2, S. 201-203, June 2000,
doi: 10.1016/S0003-682X(99)00057-2.

"""
# check inputs
if not isinstance(sample_pressure, pf.FrequencyData):
raise ValueError(
"sample_pressure has to be a pyfar.FrequencyData object")
if not isinstance(reference_pressure, pf.FrequencyData):
raise ValueError(
"reference_pressure has to be a pyfar.FrequencyData object")
if not isinstance(microphone_weights, np.ndarray):
raise ValueError("microphone_weights have to be a numpy.array")
if sample_pressure.cshape != reference_pressure.cshape:
raise ValueError(
"sample_pressure and reference_pressure have to have the "
"same cshape.")
if microphone_weights.shape[0] != sample_pressure.cshape[-1]:
raise ValueError(
"the last dimension of sample_pressure need be same as the "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"the last dimension of sample_pressure need be same as the "
"the last dimension of sample_pressure needs to be the be same as the "

"microphone_weights.shape.")
if not np.allclose(
sample_pressure.frequencies, reference_pressure.frequencies):
raise ValueError(
"sample_pressure and reference_pressure have to have the "
"same frequencies.")

# calculate according to mommertz correlation method Equation (5)
p_sample = np.moveaxis(sample_pressure.freq, -1, 0)
p_reference = np.moveaxis(reference_pressure.freq, -1, 0)
p_sample_sq = np.abs(p_sample)**2
p_reference_sq = np.abs(p_reference)**2
p_cross = p_sample * np.conj(p_reference)

p_sample_sum = np.sum(microphone_weights * p_sample_sq, axis=-1)
p_ref_sum = np.sum(microphone_weights * p_reference_sq, axis=-1)
p_cross_sum = np.sum(microphone_weights * p_cross, axis=-1)

data_scattering_coefficient \
= 1 - ((np.abs(p_cross_sum)**2)/(p_sample_sum*p_ref_sum))

scattering_coefficients = pf.FrequencyData(
np.moveaxis(data_scattering_coefficient, 0, -1),
sample_pressure.frequencies)
scattering_coefficients.comment = 'scattering coefficient'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we omitt adding the comment here, to stick more clearly to our 'comments are things the user might or might not make use of'-policy?


return scattering_coefficients


def random(
scattering_coefficients, incident_directions):
r"""Calculate the random-incidence scattering coefficient
according to Paris formula.

.. math::
s_{rand} = \sum s(\vartheta_S,\varphi_S) \cdot cos(\vartheta_S) \cdot w

with the ``scattering_coefficients``, and the
area weights ``w`` from the ``incident_directions``.
Note that the incident directions should be
equally distributed to get a valid result.

Parameters
----------
scattering_coefficients : pyfar.FrequencyData
Scattering coefficients for different incident directions. Its cshape
need to be (..., #source_directions)
incident_directions : pyfar.Coordinates
Defines the incidence directions of each `scattering_coefficients` in a
Coordinates object. Its cshape need to be (#source_directions). In
sperical coordinates the radii need to be constant. The weights need
to reflect the area weights.

Returns
-------
random_scattering : pyfar.FrequencyData
The random-incidence scattering coefficient.
"""
random_scattering = utils.paris_formula(
scattering_coefficients, incident_directions)
random_scattering.comment = 'random-incidence scattering coefficient'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above for question about using/not using comments

return random_scattering
7 changes: 7 additions & 0 deletions imkar/utils/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
from .utils import (
paris_formula,
)

__all__ = [
'paris_formula',
]
51 changes: 51 additions & 0 deletions imkar/utils/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import numpy as np
import pyfar as pf


def paris_formula(coefficients, incident_directions):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name of the function and module suggests that it is public. In this case should we add it to the docs as well?

r"""Calculate the random-incidence coefficient
according to Paris formula.

.. math::
c_{rand} = \sum c(\vartheta_S,\varphi_S) \cdot cos(\vartheta_S) \cdot w

with the ``coefficients``, and the
area weights ``w`` from the ``incident_directions``.
Note that the incident directions should be
equally distributed to get a valid result.

Parameters
----------
coefficients : pyfar.FrequencyData
coefficients for different incident directions. Its cshape
need to be (..., #incident_directions)
ahms5 marked this conversation as resolved.
Show resolved Hide resolved
incident_directions : pyfar.Coordinates
Defines the incidence directions of each `coefficients` in a
Coordinates object. Its cshape need to be (#incident_directions). In
ahms5 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Coordinates object. Its cshape need to be (#incident_directions). In
Coordinates object. Its cshape needs to be (#incident_directions). In

sperical coordinates the radii need to be constant. The weights need
to reflect the area weights.

Returns
-------
random_coefficient : pyfar.FrequencyData
The random-incidence scattering coefficient.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how common the Paris formula is, but it might be nice to add a citation here as well.

"""
if not isinstance(coefficients, pf.FrequencyData):
raise ValueError("coefficients has to be FrequencyData")
if not isinstance(incident_directions, pf.Coordinates):
raise ValueError("incident_directions have to be None or Coordinates")
if incident_directions.cshape[0] != coefficients.cshape[-1]:
raise ValueError(
"the last dimension of coefficients need be same as "
"the incident_directions.cshape.")

theta = incident_directions.get_sph().T[1]
weight = np.cos(theta) * incident_directions.weights
norm = np.sum(weight)
coefficients_freq = np.swapaxes(coefficients.freq, -1, -2)
random_coefficient = pf.FrequencyData(
np.sum(coefficients_freq*weight/norm, axis=-1),
coefficients.frequencies,
comment='random-incidence coefficient'
)
return random_coefficient
79 changes: 79 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import pytest
import pyfar as pf
import numpy as np


@pytest.fixture
def half_sphere():
"""return 42th order gaussian sampling for the half sphere and radius 1.

Returns
-------
pf.Coordinates
half sphere sampling grid
"""
mics = pf.samplings.sph_gaussian(42)
# delete lower part of sphere
return mics[mics.get_sph().T[1] <= np.pi/2]


@pytest.fixture
def quarter_half_sphere():
"""return 10th order gaussian sampling for the quarter half sphere
and radius 1.

Returns
-------
pf.Coordinates
quarter half sphere sampling grid
"""
incident_directions = pf.samplings.sph_gaussian(10)
incident_directions = incident_directions[
incident_directions.get_sph().T[1] <= np.pi/2]
return incident_directions[
incident_directions.get_sph().T[0] <= np.pi/2]


@pytest.fixture
def pressure_data_mics(half_sphere):
"""returns a sound pressure data example, with sound pressure 0 and
two frequency bins

Parameters
----------
half_sphere : pf.Coordinates
half sphere sampling grid for mics

Returns
-------
pyfar.FrequencyData
output sound pressure data
"""
frequencies = [200, 300]
shape_new = np.append(half_sphere.cshape, len(frequencies))
return pf.FrequencyData(np.zeros(shape_new), frequencies)


@pytest.fixture
def pressure_data_mics_incident_directions(
half_sphere, quarter_half_sphere):
"""returns a sound pressure data example, with sound pressure 0 and
two frequency bins

Parameters
----------
half_sphere : pf.Coordinates
half sphere sampling grid for mics
quarter_half_sphere : pf.Coordinates
quarter half sphere sampling grid for incident directions

Returns
-------
pyfar.FrequencyData
output sound pressure data
"""
frequencies = [200, 300]
shape_new = np.append(
quarter_half_sphere.cshape, half_sphere.cshape)
shape_new = np.append(shape_new, len(frequencies))
return pf.FrequencyData(np.zeros(shape_new), frequencies)
Loading