libsvm-ruby-swig icon indicating copy to clipboard operation
libsvm-ruby-swig copied to clipboard

Would be great to wrap classes into SVM:: namespace

Open dadooda opened this issue 14 years ago • 7 comments

Hi!

I'm using your SVM implementation and it seems to work.

I think it wouldn't be a bad idea to wrap Parameter, Problem and Model classes under the SVM:: namespace. Large projects may easily have these names occupied by something important.

Thank you.

Alex

dadooda avatar Sep 20 '11 10:09 dadooda

Hi Alex, I will keep that in mind next time I get time to work on this.

tomz avatar Sep 21 '11 22:09 tomz

This would be great. I'm wrapping LibSVM support into a new classifier gem and the idea of polluting downstream namespaces with unscoped class names makes me uncomfortable. Especially with model names as common as these.

dparis avatar Mar 25 '12 19:03 dparis

Namespacing would definitely be great. Glad to see you all are on top of it :)

mrgordon avatar Mar 29 '12 21:03 mrgordon

I'll do what I can. This week turned out to be unexpectedly busy for me, so it might be a few more days yet before I get around to it.

No rest for the wicked! :P

dparis avatar Mar 29 '12 21:03 dparis

Also, @tomz, namespacing the module may have compatibility implications for existing apps. At worst, I suspect any current users upgrading to a namespaced version of this gem could just do something like the following if they didn't want to go through their codebase and fix all the references:

Model = LibSVM::Model
Problem = LibSVM::Problem
...

That would make the transition fairly painless, but it would be nice to do a version bump so that ~> gemfile directives don't automatically pull in the new namespaced version into existing apps.

dparis avatar Mar 29 '12 21:03 dparis

@dparis Awesome, no rush. I'm more concerned with getting nu_svc probabilities working in the short term so I'm glad you caught the bug in #3. Just patched it locally and retraining my model now. Let me know if you all want any help packaging up fixes for pull requests.

mrgordon avatar Mar 29 '12 21:03 mrgordon

@mrgordon Glad that helped. To be clear, probabilities should work in the current code, prior to my fix, so long as parameter.probability = 1 is being set on the Parameter instance you're using before you train your Model.

The bug will only bite you if you're calling predict_probability() on a model that hasn't been trained with that parameter value set; that case should have raised an exception, which would obviously prevent any garbage data from being returned from the method, but the bug masked that problem.

dparis avatar Mar 29 '12 21:03 dparis