Julia: serialize the length of the model #2970
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While playing with the mlpack Julia bindings, I noticed that there is a bug in serialization that only happens in some niche cases.
Specifically, we serialize and deserialize the model directly to an IO stream, but we don't take the length of the serialized model into account. This means that at deserialization time, we eat everything in the stream---this screws up deserialization of anything that comes after the mlpack model.
Here's an example of a badly-behaved deserialization function:
The solution here is, luckily, pretty easy: we just also serialize a
UInt
containing the length of the serialized model, and then deserialize that first to limit ourread()
to the appropriate length:I added a test case for this situation also.