ajwheeler / ajwheeler/Korg.jl

`ews_to_stellar_parameters`: `sigma_mean` is computed from `length` of a scalar, so no √N reduction is applied

オープン
#571 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Julia
スター
67
フォーク
15
平均マージ
2日 6時間
マージ済み PR(30日)
6

説明

Hi, I found an issue in `src/Fit/fit_via_EWs.jl:560-567`:

```julia
estimated_err = std(A[isfinite.(A)])

sigma_mean = estimated_err ./ sqrt(length(estimated_err))
teff_residual_sigma = estimated_err *
get_slope_uncertainty([line.E_lower for line in linelist[neutrals]])
vmic_residual_sigma = estimated_err * get_slope_uncertainty(REWs[neutrals])

[teff_residual_sigma, sigma_mean, vmic_residual_sigma, sigma_mean]
```

`estimated_err` is a scalar, so `length(estimated_err)` is 1 and `sigma_mean`
is just `estimated_err`. Looks like the standard error of a mean was intended.

Reproduction, no Korg call needed:

```julia
using Statistics
A = randn(150) .+ 7.5
estimated_err = std(A[isfinite.(A)])
sigma_mean = estimated_err ./ sqrt(length(estimated_err))

sigma_mean == estimated_err # true
estimated_err / sqrt(length(A)) # 0.0775 vs 0.9493 — factor 12.25
```

`sigma_mean` is returned for equations 2 and 4, both differences of means:

- eq. 2, `mean(A_I) - mean(A_II)`: should be `err * sqrt(1/n_I + 1/n_II)`,
inflated ~4.2x for 150 Fe I + 20 Fe II
- eq. 4, `mean(A) - ([m/H] + A_solar)`: should be `err / sqrt(n_total)`,
inflated ~12x for 150 lines

Both feed `param_uncertainties` at line 426, so reported σ(log g) and σ([m/H])
are too large by those factors. Conservative, not dangerous — but it also makes
them incomparable with σ(Teff) and σ(vmic), which are scaled correctly through
`get_slope_uncertainty`.

The two equations have different denominators, so one `sigma_mean` cannot cover
both:

```julia
n_neutral = count(neutrals .& isfinite.(A))
n_ionised = count((.!neutrals) .& isfinite.(A))
n_total = count(isfinite.(A))

sigma_ionisation = estimated_err * sqrt(1/n_neutral + 1/n_ionised)
sigma_metallicity = estimated_err / sqrt(n_total)

[teff_residual_sigma, sigma_ionisation, vmic_residual_sigma, sigma_metallicity]
```

I can PR this. It shrinks reported σ(log g) and σ([m/H]) by roughly an order of
magnitude, so it may warrant a release note despite being a bug fix.

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。