Fix SVM types to match libsvm-js parameters

[notadamking]

Reference: https://github.com/mljs/libsvm

• What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Bug fix

• What is the current behavior? (You can also link to an open issue here)

The SVM fit, predict, and predictOne functions have incorrect TS typings, causing them to not work correctly with libsvm-js.

• What is the new behavior (if this is a feature change)?

Fix the typings to match the required parameters for libsvm-js.

• Other information:

[JasonShin]

Hi @adamjking3 ! Thank you so much for your contribution and picking up these bugs!

I think there are some minor issues in the build, which can be resolved very easily

• It is failing due to a type error in src/lib/svm/index.repl.ts. I am using *.repl.ts as a playground/demo code purposes. Please have a look at

https://ci.appveyor.com/project/JasonShin/kalimdorjs/builds/20565138/job/e42l1r9baxm1n8bi

console.log('svc2 pred ', svc2.predict([-0.8, -1])); should become console.log('svc2 pred ', svc2.predict([[-0.8, -1]]));

and
console.log('SVR predict result', svr.predict([1, 1])); should be console.log('SVR predict result', svr.predict([[1, 1]]));

Other than that, everything looks perfect! =)

[notadamking]

Hi @JasonShin, I started to fix the unit tests and the tests in the *.repl.ts file, however I ran into some issues where the libsvm-js was returning invalid results, according to their API. For example, calling svm.predictOne(array: number[]) was returning the array of numbers passed in, which is incorrect according to their documentation.

I have opened an issue, explaining the bug here mljs/libsvm#15. However, after opening the issue, I have realized that I can only reproduce the bug within this library. Creating an entirely new repo and copying their documentation produces the correct results, however copying their documentation within this repo produces invalid results. I am still looking into this to see why this is happening.

[JasonShin]

Hey @adamjking3,

For example, calling svm.predictOne(array: number[]) was returning the array of numbers passed in, which is incorrect according to their documentation.

Are you seeing this issue in the SVM unit test file? Please note that there is a Jest mock in the test file that looks like

jest.mock('libsvm-js', () => () => ({
  predict: X => X,
  predictOne: X => X,
  train: (X, y) => ({ X, y })
}));

Above code block simply bypasses libsvm.js's implementation details, which allows us to focus on testing our own code.

[notadamking]

Above code block simply bypasses libsvm.js's implementation details, which allows us to focus on testing our own code.

Ah, of course, not sure how I missed that! I will remove my silly issue from the other repo and get the unit tests fixed tonight, then update this PR.

Thanks for your help, @JasonShin

[JasonShin]

Don't forget to add yourself as a contributor using all-contributors !

[JasonShin]

Awesome!!

[JasonShin]

I will create a patch after merging this so you can start using Kalimdor =) !!

[notadamking]

I will create a patch after merging this so you can start using Kalimdor =) !!

Thanks again!

[JasonShin]

I've run all-contributors generate based on the details that you've put into .all-contributorsrc file. It wiped out @devjiro76 because he didn't put his details into the RC file.

@devjiro76 please make a new PR, cheers

[JasonShin]

@adamjking3 Congratulations on your first contribution to Kalimdor.js and welcome to the community!

[JasonShin]

Please feel free to join our Telegram chat https://t.me/joinchat/Hj2nWgsslUN0BWGi0CEOHg =)