Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mean doesn't apply weights #31

Open
Homo-carbonis opened this issue Feb 27, 2024 · 5 comments
Open

mean doesn't apply weights #31

Homo-carbonis opened this issue Feb 27, 2024 · 5 comments

Comments

@Homo-carbonis
Copy link

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
Copy link
Contributor

snunez1 commented Feb 28, 2024

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
Copy link
Author

Homo-carbonis commented Feb 28, 2024

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
Copy link
Author

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
Copy link
Contributor

snunez1 commented Feb 28, 2024

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
Copy link
Contributor

snunez1 commented Jul 30, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants