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

Fix classification error #492

Merged
merged 1 commit into from Dec 14, 2015
Merged

Fix classification error #492

merged 1 commit into from Dec 14, 2015

Conversation

pavel-zhigulin
Copy link
Contributor

Change "invVar" to "variance" matrix when calculating testProbs.

By using "invVar" you have variances product in numenator, but it needs to be in denominator.
In addition there was potential problem with accuracy when calculate exponents and then calculate logarithm.
I fixed it too.

Change "invVar" to "variance" matrix when calculating testProbs.

By using "invVar" you have variances product in numenator, but it needs to be in denominator.
In addition there was potential problem with accuracy when calculate exponents and then calculate logarithm.
I fixed it too.
@rcurtin
Copy link
Member

rcurtin commented Dec 14, 2015

I like the change of calculating the exponents in logspace, thanks. You're right about the problem, too; it looks like the test cases did not catch this. Would you like me to add your name to the list of contributors?

rcurtin added a commit that referenced this pull request Dec 14, 2015
@rcurtin rcurtin merged commit f535c29 into mlpack:master Dec 14, 2015
@rcurtin rcurtin added this to the mlpack 1.1.0 milestone Dec 14, 2015
@pavel-zhigulin
Copy link
Contributor Author

Thanx for answer.

And yes, it would be very nice from your side to add me to list of contributors. My name is Pavel Zhigulin.

@rcurtin
Copy link
Member

rcurtin commented Dec 14, 2015

Great, added in d3b7d2f. Thanks again for the contribution!

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

Successfully merging this pull request may close these issues.

None yet

2 participants