search

Eomys / MoSQITo

MoSQITo is a unified and modular development framework of key sound quality metrics favoring reproducible science and efficient shared scripting among engineers, teachers and researchers community.
Apache License 2.0
134 stars 44 forks source link

Loudness ECMA updated to 2nd Ed (2022) #78

Closed wantysal closed 8 months ago

wantysal commented 9 months ago

Following PR #74 by @fchirono , the loudness ECMA model is updated according to the revision ECMA 418-2 of december 2022. Some functions have been slightly modified to stay coherent with the rest, and the validation plots have been updated.

All tests run before PR : image

Tks, Salomé

fchirono commented 8 months ago

Hi all, thank you for accepting my contributions!

I have a question about the _ecma_time_segmentation function implementation included in this merge. The updated docstring states this function returns two variables:

However, when I run the code the variable time returned by this function is actually a list of 53 (nperseg, nseg)-shaped arrays, just like block_array. The values are all different, and it's not clear why there is such a multiplicity of values for this calculation.

(I also noticed that one of the very last lines in the loudness_ecma function is time_axis = time_array[0][:, 0], which seems to correct for this difference.)

Is there a particular reason why this implementation was adopted? When I created my implementation and submitted a pull request, I tried to keep my calculations as close to those described in the ECMA 418-2 (2nd Ed) standard as possible for the sake of clarity. Right now I can't quite understand how the merged implementation corresponds to the standard, and it's making me a bit confused!

wantysal commented 8 months ago

Hi Fabio,

The version implemented in this PR gives the exact same block arrays as yours. We just deleted a "for" loop to vectorize the code for a better optimization. Concerning the time array, as you mentioned in your implementation, there is no indication in the standard of which time value to use for each block (mean ? first value of the block ? l value ?). For debug purpose it had been chosen to return the complete axis.

Reading your comment, we decided to modify the code to keep only the mean value as you suggested if it is clearer. Thank you for your remark, please note that we updated the docstring aswell to indicate the choice done ;)

Salomé

fchirono commented 8 months ago

Hi Salome; this makes sense, thank you for clarifying your approach!