search

Lisp-Stat / lisp-stat

Lisp-Stat main system
https://lisp-stat.github.io/lisp-stat
Microsoft Public License
145 stars 11 forks source link

mean doesn't apply weights #31

Open Homo-carbonis opened 8 months ago

Homo-carbonis commented 8 months ago

I don't know if this is a bug per se but I was confused to find (lisp-stat:mean #(1.0 2.0) :weights #(1.0 100.0)) returns 1.5. As I understand it, lisp-stat:mean falls back to alexandria:mean if it isn't passed a vector with element-type set to fixnum, single, double or bool. alexandria:mean doesn't accept a weight parameter so the weights are silently discarded.

Is there a reason nu.statistics:mean isn't used for everything? It seems to be able to handle any sequence of numbers.

Failing that I think it should signal an error instead of falling back to alexandria when weights are supplied.

snunez1 commented 8 months ago

Well, that's one of the better questions I've seen in a while. I don't think we should silently discard the weights. At least a warning should be printed to alert the user to what happened. I can't remember now why we aren't using nu:mean for everything. For a while we were, and then there was a reason to change it. That might not be true any longer.

At the moment I'm teaching a class that's full-time, face-to-face, and don't have much time to devote to looking at this. The class ends mid-May and I can circle back then. However if you feel like looking through this and coming up with a good alternative (it seems you are on your way), I'd happily accept a pull request around refactoring our processing of mean.

Homo-carbonis commented 8 months ago

Okay, I'll have a look. There is a comment which says ";prefer alexandria because we want to remove redundant functions from LH stats" but I presume that just refers to the t case.

Homo-carbonis commented 8 months ago

I've had a look but I can't work out why nu isn't used for everything. Should I assume there is a good reason and add an (assert (not weights)) before falling back to alexandria?

I also noticed the biased? parameter to variance has the reverse problem. It's only used with alexandria and not with nu.

snunez1 commented 8 months ago

Ah, the biased? thing is starting to sound familiar now. I seem to recall it was somehow related to a difference between the two. It might be worth investigating that a bit further. If we can't use nu everywhere, then the assert is a good way to go.

snunez1 commented 3 months ago

Hello @Homo-carbonis, catching up on the issues. What did you end up doing about the weights with the mean calculation?