Please help with colour.sd_gaussian definition output.

[EricFlyMeToTheMoon]

I'm using "colour.sd_gaussian" for some project. I found it is not generating correct Gaussian distribution.

Here is my test code:

import colour
peak_wl = 555
fwhm = 20
sd1 = colour.sd_gaussian(peak_wl, fwhm, method="FWHM")
print(sd1[peak_wl])
print(sd1[peak_wl - fwhm/2])

1.0
0.778800783071

Based on how FWHM is defined, I think it should be 0.5 instead of 0.7788. Please help.

[KelSolaar]

Hi @EricFlyMeToTheMoon,

I'm abroad with limited time to look at things, we follow and implement Yoshi Ohno formula, he refers to FWHM as half spectral width:

Cheers,

Thomas

[EricFlyMeToTheMoon]

Hi Thomas,

My understanding is: Yoshi Ohno formula is a special fitting function in order to better match typical LED emission spectrum (typical Gaussian distribution doesn't match typical LED emission very well). Yoshi Ohno formula is quite different from Gaussian.

However, if "colour.sd_gaussian" is defined as "Returns: Gaussian spectral distribution.", it should use the Gaussian distribution instead of Yoshi Ohno formula, right?

Thanks for the kind reply.

Best,
Eric

[tjdcs]

I agree with Eric here. That paper excerpt refers to the "half width" whereas our api uses "full width half max". The ohno excerpt could be understood to be roughly the "half width half max".

Solutions could be to rename our api to half width
add both fwhm and hwhm, and defer to hwhm by default / argument order. That way non-specific user code will remain the same, but using fwhm kwarg will be mathematically correct.

[KelSolaar]

Ah yes you are both correct, some other options:

• Rename sd_gaussian_fwhm to sd_half_spectral_width and the argument to hwhm, this would NOT break the API.
• Keep sd_gaussian_fwhm but fix it to actually use fwhm and add a new sd_half_spectral_width definition, this would break existing behaviour but it seems like we have an issue here so a fix is in order.

[tjdcs]

Also if we don't have that LED model in the code, it could be very helpful. - Tucker

…

On Tue, Jun 27, 2023 at 3:04 PM Thomas Mansencal ***@***.***> wrote: Ah yes you are both correct, some other options: - Rename sd_gaussian_fwhm to sd_half_spectral_width and the argument to hwhm, this would NOT break the API. - Keep sd_gaussian_fwhm but fix it to actually use fwhm and add a new sd_half_spectral_width definition, this would break existing behaviour but it seems like we have an issue here so a fix is in order. — Reply to this email directly, view it on GitHub <#1171 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABS4J5WBUXOFSNDAMNM3QC3XNMVC7ANCNFSM6AAAAAAZQ3O5F4> . You are receiving this because you commented.Message ID: ***@***.***>

[EricFlyMeToTheMoon]

Just some update here.

I tested "colour.sd_gaussian" with sigma (sigma = fwhm / 2.35482) as the parameter instead of FWHM; it seems generating correct Gaussian distribution in this way.

But if using FWHM as the parameter, it is not generating correct Gaussian distribution.

Anyway, I'm a new user here and I think this is a great python package. And thanks all for building it.

Regards,
Eric

import colour

peak_wl = 555
fwhm = 20

sd1 = colour.sd_gaussian(peak_wl, fwhm, method="FWHM")
print(sd1[peak_wl])
print(sd1[peak_wl - fwhm/2])

sigma = fwhm / 2.35482

sd2 = colour.sd_gaussian(peak_wl, sigma, method="Normal")
print(sd2[peak_wl])
print(sd2[peak_wl - fwhm/2])

1.0
0.778800783071
1.0
0.500000013255

[EricFlyMeToTheMoon]

Guys, I think this is a bug, after reading the source code.

The function sd_gaussian_fwhm is using a wrong formula of generating Gaussian distribution.

Gaussian distribution (https://mathworld.wolfram.com/GaussianFunction.html) should not directly use FWHM as input, but need to convert FWHM to Sigma first.

[KelSolaar]

Fixed with #1184, merging soon!

[EricFlyMeToTheMoon]

Thank you and greatly appreciated!

[KelSolaar]

My pleasure, sorry for the wait!