Would be great to wrap classes into SVM:: namespace

Question

Would be great to wrap classes into SVM:: namespace

Opened this issue 13 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

Answer 1 · 2011-09-21T22:27:34.000Z

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

Answer 2 · 2012-03-25T19:06:44.000Z

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.

Answer 3 · 2012-03-29T21:04:01.000Z

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

Answer 4 · 2012-03-29T21:06:09.000Z

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

Answer 5 · 2012-03-29T21:11:47.000Z

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.

Answer 6 · 2012-03-29T21:26:21.000Z

@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.

Answer 7 · 2012-03-29T21:40:37.000Z

@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.