RAP Lateral Level based on energy ratio#143
Conversation
SimonBuechner
moved this from Implementation in progress
to Require review
in Weekly Planning
SimonBuechner
requested review from
a team,
ahms5,
artur-pa,
f-brinkmann,
hoyer-a and
mberz
SimonBuechner
moved this from Require review
to Implementation in progress
in Weekly Planning
pyrato/parameters.py
Outdated
|
|
||
| return energy_ratio | ||
|
|
||
| def late_lateral_level(energy_decay_curve_ref_10m, |
There was a problem hiding this comment.
| def late_lateral_level(energy_decay_curve_ref_10m, | |
| def late_lateral_level_difference(energy_decay_curve_ref_10m, |
It's still a ratio on linear scale and difference on log scale isn't it?
If feel that the names should be consistent among the different parameters, see early lateral ratio function name.
There was a problem hiding this comment.
Doesn't the 'level' already indicate that it is log scale? Or do you mean something else?
There was a problem hiding this comment.
ISO 3382 referrs to this as 'late lateral sound level' (Table A.1) and 'lateral sound energy level' (above Eq. A.17). I would suggest to use the same terminology as in the standard and use the former for brevity. Although, I agree that 'level difference' would be more precise.
There was a problem hiding this comment.
I think it would make sense to name the function identical to the norm. I've changed that in this PR.
github-project-automation
bot
moved this from Implementation in progress
to Require review
in Weekly Planning
f-brinkmann
left a comment
f-brinkmann
left a comment
There was a problem hiding this comment.
Thanks, the code looked fine. Only comments on docs and function name.
pyrato/parameters.py
Outdated
|
|
||
| return energy_ratio | ||
|
|
||
| def late_lateral_level(energy_decay_curve_ref_10m, |
There was a problem hiding this comment.
ISO 3382 referrs to this as 'late lateral sound level' (Table A.1) and 'lateral sound energy level' (above Eq. A.17). I would suggest to use the same terminology as in the standard and use the former for brevity. Although, I agree that 'level difference' would be more precise.
SimonBuechner
force-pushed
the
feature/lateral_level
branch
from
1a1e042 to
ee2ee38
Compare
|
@SimonBuechner There are quite some conflicts. Since it's so many I think it makes sense to resolve them before reviews are done. |
Co-authored-by: Fabian Brinkmann <fabian.brinkmann@mailbox.org>
…ocstring adjustment
SimonBuechner
force-pushed
the
feature/lateral_level
branch
from
ee2ee38 to
80b7521
Compare
Should be solved now. |
artur-pa
left a comment
artur-pa
left a comment
There was a problem hiding this comment.
Looks good, I have two small comments.
| Energy decay curve of the reference free field impulse response | ||
| measured vwith an omnidirectional microphone at 10 m distance in the | ||
| free field. The EDC must start at time zero. |
There was a problem hiding this comment.
Do we want to be consistent with the sound strength parameter?
energy_decay_curve_free_field : pyfar.TimeData
Energy decay curve of the reference free-field impulse response
at 10 m. The EDC must start at time zero.
There was a problem hiding this comment.
This comment is resolved, isn't it?
| Energy decay curve of the reference free field impulse response | ||
| measured vwith an omnidirectional microphone at 10 m distance in the | ||
| free field. The EDC must start at time zero. |
There was a problem hiding this comment.
This comment is resolved, isn't it?
github-project-automation
bot
moved this from Require review
to Done
in Weekly Planning
4 participants
Changes proposed in this pull request: