Add serialization functionality - #104
Conversation
|
Pipeline fails because OpenFHE will expose SerializeToString and DeserializeFromString only when hpsc-lab/openfhe-julia#129 is implemented. |
|
@sloede this is ready for a first review despite the failing pipeline, see comment above. |
…d unit tests and benchmark for serialization
|
Note that the implementation might have to change very soon again, see openfheorg/openfhe-development#1191. We should thus discuss if we still want to use the effectively deprecated API or rather find a "proper" solution to serializing/deserializing right now. |
|
As I understand it, openfheorg/openfhe-development#1191 removes only the override for cryptocontext serialization which is implemented in cryptocontext-ser.h, this should not be a problem. The implementation in serial.h seems to be identical, so wen can already leave out the override for cryptocontext on our side in anticipation of this change. |
|
ToDos:
|
Rename `serialize` → `serialize_to_json_string` and `deserialize` → `deserialize_from_json_string` to avoid overwriting `Serialization.serialize`/`Serialization.deserialize` from Julia's stdlib. Add experimental warnings to serialization docstrings and cross-references between string-based and file-based variants.
- overload julia stdlib serialization and - work directly with SecureArithmetic objects
|
@sloede I reworked the serialization implementation so it overloads Julias Serialization.jl functions serialize and deserialize. At the moment, this serialized OpenFHE objects to JSON strings in the background. This does not make a lot of sense since the output is still not human readable, because we have to serialize the json string with Julias standard serialization function and add additional information to the serialized stream. This makes it non-human-readable, thus json does not make any sense for this application. We could save a lot of space by switching to binary serialization on the OpenFHE side. As we saw, they only implement binary serialization is serialize to file. We could mitigate this by passing std::ostringstream to lbcrypto::Serial::Serialize instead of a file handle like openfhe does internally when calling SerializeToFile. If we still want to support serialization to JSON for human readability: we could use JSON.jl to serialize to and from JSON. We would have to wrap the json we get from openfhe, because the Secure Arithmetic serializations need additional information like the cryptocontext, shape and capacity. |
|
Thanks a lot for the summary. Let's put the focus on creating an MVP first, then optimizing it later. Thus I'd say if the current PR already supports what we want to do (albeit less efficiently), then let's move forward and create an issue for the discussed potential improvements. |
|
@sloede I just created a new release for openfhe-julia which includes the code for exposing the serialization functionality. This new functionality never touches OpenFHE.jl. But SecureArithmetic only depends on OpenFHE.jl, not openfhe_julia. But this particular feature requires a minimum version of openfhe_julia (the new v0.6.1) to work. We cannot simply add openfhe_julia as a dependency, because of the compatibility issues we are trying to solve with hpsc-lab/OpenFHE.jl#123. I see three options:
|
|
SA.jl should only depend on OpenFHE.jl and be shielded from any upstream shenanigans. AFAICT, releasing the new version of OpenFHE.jl with compat bounds on both openfhe-julia and OpenFHE_jll would allow us to just bump the lower bound for OpenFHE.jl in SA.jl's |
I agree.
Yes, but only if we also bump the minimum required openfhe-julia version in the new OpenFHE.jl release to v0.6.1. Plan:
|
Use loop instead of map Co-authored-by: Michael Schlottke-Lakemper <michael@sloede.com>
…or experimental state of serialization functions
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
|
@sloede CI already passes with current openfhe-julia release, the OpenFHE.jl release is on its way with hpsc-lab/OpenFHE.jl#126. |
|
Before I review, please address my open comments above. If you fixed/addressed them, please mark them as resolved (with our without commenting). If they are not relevant anymore, just say so in the comment and mark as resolved. If you are unsure how to respond, just ask :-) If everything is resolved (or responded to), please ping me again. |
|
@sloede I marked everything as resolved except the unit tests for the benchmark script. We can either remove the script or add a smoke test, but proper unit tests do not seem fitting. |
Thanks!
I think running this as part of the tests, but rather a as a smoke test (i.e., it runs without warnings), would be good. As I said before, you should also add the |
Co-authored-by: Michael Schlottke-Lakemper <michael@sloede.com>
2 participants
No description provided.