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 <#267 (comment)>, 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 <#267 (comment)>, 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 <#267 (comment)>, 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.