Cannot perform inference on deserialized SVC and SVR

[Roee-87]

I'm submitting a

• [ ] bug report.
• [ ] improvement.
• [ x] feature request.

Current Behaviour:

SVC and SVR models skip the parameters during serialization. What is the correct way to deserialize a model in order to perform inference?

let lr_json = serde_json::to_string(&my_model);

produces the following json file:

{ "classes": [-1, 1], "instances": [ [4.9, 2.4, 3.3, 1.0], [4.4, 2.9, 1.4, 0.2], [7.0, 3.2, 4.7, 1.4], [5.5, 2.3, 4.0, 1.3], [6.9, 3.1, 4.9, 1.5], [5.4, 3.9, 1.7, 0.4], [5.7, 2.8, 4.5, 1.3], [6.3, 3.3, 4.7, 1.6] ], "w": [ 0.7330568313232404, -0.9886291197226762, 0.4983642540817094, 0.1369267334839056, 0.12914908421939392, -0.9760276163690054, 0.3306627164164916, 0.13649711656694088 ], "b": 0.18156339068191985, "phantomdata": null }

The "parameters" field is missing due a serde skip command in svc.rs:128:7.

A deserialized model cannot perform inference using SVC.predict() due to the missing parameters field.

let y_hat: &Vec<f64> = &model.predict(&x).unwrap();

results in an error:

thread 'main' panicked at 'called Option::unwrap() on a None value', /Users/.../.cargo/registry/src/index.crates.io-6f17d22bba15001f/smartcore-0.3.2/src/svm/svc.rs:349:26

Expected Behaviour:

If serde skip is removed and I am able to serialize the parameters, I would expect a deserialized model to perform inference.

Steps to reproduce:

https://github.com/Roee-87/SVC/blob/master/src/main.rs

[Mec-iS]

Hi, thanks for using Smartcore. Could you provide an example with current output and the output you expected?

[Roee-87]

Yes. When performing inference using a deserialized SVC model, I obtain the following error:

thread 'main' panicked at 'called Option::unwrap() on a None value'

This is due to the fact when serializing the model, the "parameters" field of the SVC object is not included due to a serde skip instruction in svc.rs:128:7. Calling SVC.predict() on a deserialized model causes an error in svc.rs:349:26 due to non-existence of the parameters field.

Here is an example of what an attempt at serializing an SVC model looks like: https://github.com/Roee-87/SVC/blob/master/SVM_model.json

[Roee-87]

Hi Lorenzo,

I'm interested in exporting a trained SVM classifier. Is there a reason that the parameters section is skipped over https://github.com/smartcorelib/smartcore/blob/dbdc2b2a77d7b73cc483f238b44382946a785f6a/src/svm/svc.rs#L128 during serialization?

Thank you! -Roy

On Mon, Aug 14, 2023 at 1:11 AM Lorenzo @.***> wrote:

Hi, thanks for using Smartcore. Could you provide an example with current output and the output you expected?

— Reply to this email directly, view it on GitHub https://github.com/smartcorelib/smartcore/issues/267#issuecomment-1676881834, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2XWC3NW6LKVY5O4ZWV2UXTXVHMS3ANCNFSM6AAAAAA3PFR7IM . You are receiving this because you authored the thread.Message ID: @.***>

[Mec-iS]

yes unfortunately we never managed to provide proper serialization for SVCParameters. Honestly I don't remember at the moment what the issue was. If I remember correctly Kernel is basically a function and it cannot be serialised.

[Roee-87]

Ah, that makes sense! A working solution in my fork -- albeit somewhat brute force and inelegant -- is to make the params field public and manually insert the correct SVCParameters back into the deserialized model struct. Running .predict() on the updated model works as expected.

On Wed, Aug 23, 2023 at 10:36 AM Lorenzo @.***> wrote:

yes unfortunately we never managed to provide proper serialization for SVCParameters. Honestly I don't remember at the moment what the issue was. If I remember correctly Kernel is basically a function and it cannot be serialised.

— Reply to this email directly, view it on GitHub https://github.com/smartcorelib/smartcore/issues/267#issuecomment-1690368146, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2XWC3JLKEBBZ4CLTTWM3JTXWY5RBANCNFSM6AAAAAA3PFR7IM . You are receiving this because you authored the thread.Message ID: @.***>

[Mec-iS]

you can open a Pull Request and I can take a look to it, please provide a test also

[Roee-87]

I can submit a PR, but this approach IMO would only be safe for development purposes and I wouldn't recommend merging it. An alternative solution that I can work on (and would take a few weeks) would be to replace the Kernel field with an enum and then pattern match with the correct kernel function.

On Thu, Aug 24, 2023 at 9:41 AM Lorenzo @.***> wrote:

you can open a Pull Request and I can take a look to it, please provide a test also

— Reply to this email directly, view it on GitHub https://github.com/smartcorelib/smartcore/issues/267#issuecomment-1692047745, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2XWC3PR5SGWPFV4WD6GDOLXW5745ANCNFSM6AAAAAA3PFR7IM . You are receiving this because you authored the thread.Message ID: @.***>

[Mec-iS]

replace the Kernel field with an enum and then pattern match with the correct kernel function.

that would be great! take the time needed, as a project we aim for stability and consistency. Will be glad to review the PR.