added lightning.KernelSVC (binary only)

[StrikerRUS]

Refer to #172 (comment). KernelSVC is the last model from lightning library that can be added to m2cgen.

Adding only binary classification to not make PR too huge.

[StrikerRUS]

Have no idea how to avoid zero norm 🙁

[izeigerman]

What is the lighting's behavior for zero feature vector norm? Can we get away with the same logic that we use for Support Vector norm?

[StrikerRUS]

It relies on scikit-learn code:
https://github.com/scikit-learn/scikit-learn/blob/95d4f0841d57e8b5f6b2a570312e9d832e69debc/sklearn/metrics/pairwise.py#L1169-L1173
https://github.com/scikit-learn/scikit-learn/blob/95d4f0841d57e8b5f6b2a570312e9d832e69debc/sklearn/preprocessing/_data.py#L1730
https://github.com/scikit-learn/scikit-learn/blob/95d4f0841d57e8b5f6b2a570312e9d832e69debc/sklearn/preprocessing/_data.py#L63-L78

[izeigerman]

If I understand the logic correctly it does the same thing (falls back to 1.0), shall we do the same here?

[StrikerRUS]

To be honest, I'm not sure that complicating generated code in a such way worth it...

[izeigerman]

I'd say we should always prefer correctness even if it means a slightly more complex code. Btw, does it really become that complex? Especially considering that we reuse the expression (to_reuse=True) here. AST doesn't look bad at all. Thanks for implementing this! 👍

[StrikerRUS]

we should always prefer correctness even if it means a slightly more complex code.

Yeah, no doubt!

In languages which implement to_reuse=True it looks OK, but if they don't... Though, there are many other places where without caching code looks terrible.

I thought about more elegant solution, but didn't come to any.

[izeigerman]

I thought about more elegant solution, but didn't come to any.

Well, TBH, the existing solution looks more than good to me at least at this point 👍

[coveralls]

Coverage decreased (-0.3%) to 95.522% when pulling 10a7e46 on kernelsvc into c342ca1 on master.

[izeigerman]

What is the lighting's behavior for zero feature vector norm? Can we get away with the same logic that we use for Support Vector norm?

[izeigerman]

Fantastic addition to the catalog, thank you 👍 A few minor comments

[izeigerman]

Let's add a fallback logic for the feature norm and this is good to go. Thanks for addressing comments 👍

[izeigerman]

[Nitpick] This part can be extracted into a separate variable and cached, this way making expected AST a little simpler:

feature_norm = ast.SqrtExpr(
                            ast.BinNumExpr(
                                ast.FeatureRef(0),
                                ast.FeatureRef(0),
                                ast.BinNumOpType.MUL),
                            to_reuse=True)

...
ast.IfExpr(
                    ast.CompExpr(
                        feature_norm,
                        ast.NumVal(0.0),
                        ast.CompOpType.EQ),
                    ast.NumVal(1.0),
                    feature_norm)

[StrikerRUS]

Sure, will do this in a separate PR.