Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds protobuf serialization for LibLinear models #273

Merged
merged 1 commit into from
Sep 20, 2022

Conversation

Craigacp
Copy link
Member

Description

Adds protobuf serialization for liblinear models. Unfortunately rebuilding a LibLinear Model requires reflective access into the model as the fields are all package private with no setters, so we resort to using a ByteArrayOutputStream to serialize the model using Java serialization before packing it into a protobuf. The protobufs do exist for doing it with reflection, but it'll cause issues with the module system so it's not turned on.

Motivation

We need to move Tribuo off Java serialization.

@Craigacp Craigacp added the squash-commits Squash the commits when merging this PR label Sep 10, 2022
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Sep 10, 2022
Copy link
Member

@pogren pogren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

straightforward protobuf serialization has good test code coverage. I don't understand the purpose of the new methods in LibLinearModel: serializeModel, deserializeModels, and setField which have zero references and zero test code coverage.

@Craigacp Craigacp merged commit b35827b into main Sep 20, 2022
@Craigacp Craigacp deleted the liblinear-serialization branch September 20, 2022 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement. squash-commits Squash the commits when merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants