Skip to content
This repository has been archived by the owner on Dec 22, 2018. It is now read-only.

stat/distuv: incorrect length of suffStat parameter passed Normal.Fit #177

Closed
dongweigogo opened this issue Apr 20, 2017 · 2 comments
Closed

Comments

@dongweigogo
Copy link

dongweigogo commented Apr 20, 2017

For normal distribution, the suffstat obviously has a length of 2, with mu and sigma. But in the distuv/norm.go source code, the length of suffStat in the Fit method is set to 1. That causes a panic.

func (n *Normal) Fit(samples, weights []float64) {

    suffStat := make([]float64, 1)                  //HERE

    nSamples := n.SuffStat(samples, weights, suffStat)

    n.ConjugateUpdate(suffStat, nSamples, []float64{0, 0})

}

I think that should be set to 2.

@dongweigogo dongweigogo changed the title A wrong setting of Fit() in Normal distribution A incorrect setting of suffStat in Fit() of Normal distribution Apr 20, 2017
@dongweigogo dongweigogo changed the title A incorrect setting of suffStat in Fit() of Normal distribution An incorrect setting of suffStat in Fit() of Normal distribution Apr 20, 2017
@kortschak
Copy link
Member

Presumably it should be written

func (n *Normal) Fit(samples, weights []float64) {
        suffStat := make([]float64, n.NumSuffStat())
        nSamples := n.SuffStat(samples, weights, suffStat)
        n.ConjugateUpdate(suffStat, nSamples, make([]float64, n.NumSuffStat()))
}

I would think NumSuffstat deserves some documentation too.

Also fix Exponential with the same code.

@kortschak
Copy link
Member

kortschak commented Apr 21, 2017

@dongweigogo Please send a minimal reproducer for testing.

@kortschak kortschak changed the title An incorrect setting of suffStat in Fit() of Normal distribution stat/distuv: incorrect length of suffStat parameter passed Normal.Fit Apr 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants