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

Bug in ruby Model class' predict_probability() method #3

Open
dparis opened this issue Mar 25, 2012 · 5 comments
Open

Bug in ruby Model class' predict_probability() method #3

dparis opened this issue Mar 25, 2012 · 5 comments
Assignees

Comments

@dparis
Copy link

dparis commented Mar 25, 2012

At https://github.com/tomz/libsvm-ruby-swig/blob/master/libsvm-3.1/ruby/svm.rb#L280:

#only C_SVC,NU_SVC goes in
if not @probability
    raise TypeError, "model does not support probabiliy estimates"
end

You're assuming that @probability will be false if it's not set, but from what I can tell the valid values are integer 1 and 0. Ruby does not consider zero to evaluate to boolean false.

This was a pain to track down; my codebase had a bug where I thought probabilities had been enabled on the model, but it wasn't actually being set. Since the TypeError is never raised in that case, the probability values are returned as if everything is fine. However, since the underlying LibSVM model doesn't have valid probability values defined, the C code returns garbage unallocated data cast into floats as the probability values to ruby. This was deeply confusing as I initially didn't know that the data was unreasonable until I started working with the results more closely. :(

@tomz
Copy link
Owner

tomz commented Mar 26, 2012

thanks dparis for finding this, looks like changing line 212 to the following will fix it?

@probability = (svm_check_probability_model(@model) == 1)

@ghost ghost assigned tomz Mar 26, 2012
@dparis
Copy link
Author

dparis commented Mar 26, 2012

Yes, that should work.

While I've got your attention, is there any chance you could republish the most recent version to rubygems? The version on rubygems is still using LibSVM 2.9.

As well, I've also got some feature requests that should be fairly straightforward. If you are going to update the published gem, would you mind waiting until I can get a pull request to you with the new feature additions? They're mostly to expose a few more properties or methods through the Model interface, nothing critical but it would allow me to clean up my code a bit. I could even take a look at fixing #2 while I'm at it.

@tomz
Copy link
Owner

tomz commented Mar 26, 2012

Ok sounds good, feel free to send your pull requests when you are ready. Looks like I've already upgraded the github version to LibSVM 3.1(I probably forgot to publish it), I think the latest is 3.1.1, I will get that in when I get a chance.

@dparis
Copy link
Author

dparis commented Mar 26, 2012

Excellent. I'll try to have those sent out in the next day or two, time permitting.

@dparis
Copy link
Author

dparis commented Apr 5, 2012

I haven't forgotten about this, just been extra busy this last week or so. I'm hoping to have some time late tonight and tomorrow to get around to this stuff.

Cheers!

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