Serialization using cereal #1704

isuruf · 2020-11-06T11:31:52Z

TODO:

rename serialize.h -> serialize-cereal.h
Add loads, dumps methods to basic.h and implement it using cereal in basic.cpp
Add tests
Make compiling conditional on cereal being available
Implement more classes.
~~Use something else than typeid. They are not stable across versions.~~

isuruf-bot · 2020-11-06T12:30:31Z

Hi,

I've run clang-format and found that the code needs formatting.
Here's a commit that fixes this. isuruf-bot@2d24e7a

To use the commit you can do

curl -o format.diff https://github.com/isuruf-bot/symengine/commit/2d24e7abba25af146bd2221f59c59d7947c9a21d.diff
git apply format.diff

isuruf · 2020-11-07T17:09:12Z

symengine/serialize-cereal.h

+
+#define SYMENGINE_ENUM(type, Class)                     \
+    template <class Archive>                            \
+    inline void save_basic(Archive &ar, const Class &b) \


You can use save_basic(Archive &ar, const OneArgFunction &b) and get rid of defining these multiple times.

Ah, of course, will fix this.

So this really reduced the number of save_basic implementations. For load_basic I haven't yet come up with anything as elegant. I have the gut feeling it should be possible to do something leaner, any ideas?

load_basic(Archive &ar, RCP<const OneArgFunction> &b) ?

OneArgFunction is abstract, so I don't think that's possible?

I mean I can dispatch on it, but after that I'm at loss:

template <class Archive> RCP<const Basic> load_basic(Archive &ar, RCP<const OneArgFunction> &) { RCP<const Basic> arg; ar(arg); return basic_rcp_from_type_code_and_args(type_code, arg); }

I guess I could change the signature of all load_basic dispatch functions to take type_code as an arguemnt, but then we still need the hypothetical basic_rcp_from_type_code_and_args?

See f7ada37

bjodah · 2020-11-07T17:10:33Z

OK, I'm done for tonight (GMT+1 in Stockholm). If you get the chance to take a look: let me know what you think so far ☺.

isuruf · 2020-11-07T17:10:32Z

symengine/serialize-cereal.h

+template <class Archive, typename T>
+RCP<const Basic> load_basic(Archive &ar, RCP<const T> &b)
+{
+    const auto t_code = b->get_type_code();


b is a dummy to allow writing code with inheritance. Don't use it.

Sure, but isn't it guaranteed that it inherits from Basic?

Yes, but the pointer inside might be NULL, so you can't call methods on it.

isuruf · 2020-11-07T17:11:37Z

symengine/basic.cpp

@@ -24,6 +43,20 @@ std::string Basic::__str__() const
    return str(*this);
 }

+std::string Basic::dumps() const {
+    std::ostringstream oss;
+    cereal::BinaryOutputArchive{oss}(this->rcp_from_this());


Is BinaryOutputArchive portable?

Thanks, no it wasn't: endianness is handled by PortableBinaryOutputArchive. I just changed this.

bjodah · 2020-11-08T13:53:40Z

As for TypeIDs not being stable across versions of symengine: Cereal supports versioned serialization. We could write one new migrator for each new version of SymEngine, translating TypeIDs from version x to version x + 1. No performance impact for users keeping their serialized data up to date, and probably an acceptable cost for users using old data?

bjodah · 2020-11-08T16:44:00Z

Ah, nice, std::is_base_of & std::enable_if, so this is SFINAE in action I presume? (I never seem to get it right whenever I try to use those).

isuruf · 2020-11-08T16:56:02Z

Yes, it is SFINAE

bjodah · 2020-11-08T22:14:57Z

Thanks, I got rid of those helpers I introduced before your SFINAE design.

So debugging SFINAE is a bit of a pain right? I'm trying to support URatPoly but it's not picking up my save_basic overload:

template <class Archive>
inline void save_basic(Archive &ar, const URatPoly &b)
{
    ar(b.get_var());
    ar(b.get_poly());
}

Excerpt from test_lambda_double

Evaluate functions
-------------------------------------------------------------------------------
../symengine/tests/eval/test_lambda_double.cpp:187
...............................................................................
../symengine/tests/eval/test_lambda_double.cpp:187: FAILED:

{Unknown expression after the reported line}

due to unexpected exception with message:

../symengine/serialize-cereal.h:23: void SymEngine::save_basic(Archive&,

const SymEngine::Basic&) [with Archive = cereal::PortableBinaryOutputArchive]

not supported: URatPoly (15), tan(x)*sec(x)

I guess some one or several of my overloads for the building blocks here (URatDict -> rational_class -> integer_class) isn't quite working. But I don't know how to debug this behavior, any tips?

isuruf · 2020-11-08T22:24:56Z

Don't know how to debug the general case. In this case you probably need Mul

rikardn

This is great work!

One question: What would happen if attempting to serialize an object that isn't (yet) supported?

benchmarks/.gitattributes

rikardn · 2021-11-07T16:52:49Z

symengine/serialize-cereal.h

@@ -0,0 +1,696 @@
+#pragma once


This is non-standard. Use include guards instead?

I'm curious: can you name a compiler which can compile symengine, but doesn't support #pragma once? 🙂

I've seen the #pragma once used lately in C++. So far I always suggested to just use the old fashion include guards.

I am not against using #pragma once, it is definitely simpler. However I want to make sure we do not break the compilation.

@bjodah can you summarize the support in compilers for #pragma once? And why is it not standard?

Good point. The only compiler I've seen is Oracle Development Studio which we have partial support for. #1646

I don't know why it's not supported by the standard (the standard doesn't say much about the preprocessor from what I've seen). My question was genuine: I was curious about the existence of any compilers which lacks support for it (and I've seen multiple recommendations to prefer the pragma approach). But now we know: Oracle doesn't support it then. 🙂

Do we want to support Oracle?

It's not a big deal to keep using include guards instead of pragma once. I think we are using Oracle for R wrappers.

Let's use the guards then.

isuruf · 2021-11-07T17:37:20Z

One question: What would happen if attempting to serialize an object that isn't (yet) supported?

A SymEngine::NotImplementedError exception is thrown.

bjodah · 2021-11-07T19:16:16Z

Do we want to add a .gitattributes file so that vendored files from cereal doesn't show up in the "Files changed view"?, I think a .gitattributes file with an entry such as this would work:

symengine/utilities/cereal/include/cereal/* linguist-vendored

bjodah

Great stuff! I only spotted two potential minor issues.

symengine/utilities/stream_fmt.h

LICENSE

isuruf added 4 commits November 6, 2020 04:11

Initial serialization implementation using cereal

ff14bdf

test serialize

efa0c1e

Add and Integer

750a089

remove temporary

f3ee326

isuruf mentioned this pull request Nov 6, 2020

Serializing SymEngine objects #1394

Closed

Scaffolding tests of serialization

8423c77

bjodah added 6 commits November 7, 2020 13:27

fix test_serialize

76708c7

Add Basic::dumps() -> string & Basic::loads(const string&)

a811c12

SymEngine::type_code_name(TypeID) -> string

701a2d4

More informative error message

9c0d2fd

Use test_lambda_double to exercise dumps() -> loads() round-trip

928a66a

Split type_codes.inc into subfiles

fbf4be0

isuruf commented Nov 7, 2020

View reviewed changes

bjodah added 3 commits November 8, 2020 12:26

RFC: supporting OneArgFunction

19ebf8f

Use PortableBinaryOutputArchive in Basic::dumps and Basic::loads

6f83979

Handle Relational in load_basic

ca3a82a

isuruf added 2 commits November 8, 2020 10:30

use templates for onargfunction

f7ada37

formatting

3b957fc

bjodah added 2 commits November 8, 2020 22:20

Remove static methods no longer needed

68c9365

Attempting to support URatPoly

ae98a51

Add support for Mul

5fbd488

isuruf added 7 commits November 6, 2021 16:40

install serialize-cereal.h

b34357b

need <array> for std::array

bb19169

fix missing cxxabi.h

de3cc6f

__PRETTY_FUNCTION__ is not on msvc

e03a51d

add missing array

5d842d2

clang-format

27765c4

Add a hack for piranha and gmpxx

bc0832f

isuruf force-pushed the cereal branch from 30a5105 to bc0832f Compare November 7, 2021 07:05

isuruf added 6 commits November 7, 2021 00:43

disable cotire

09412a5

hack only for gmpxx

b8afe74

custom method for URatDict

87bee85

run clang-format

6f703d6

avoid SFINAE which might go horribly wrong

73615b1

Add missing iterator

57350b3

isuruf requested a review from a team November 7, 2021 16:55

rikardn approved these changes Nov 7, 2021

View reviewed changes

isuruf added 2 commits November 7, 2021 09:03

remove benchmarks/.gitattributes

f36e6f9

remove pragma once

93a79df

isuruf added 2 commits November 7, 2021 11:34

mark cereal sources as vendored

d3a5831

fix gitattributes

1fb9c95

isuruf requested review from certik, rikardn and bjodah November 9, 2021 20:42

bjodah approved these changes Nov 9, 2021

View reviewed changes

symengine/utilities/stream_fmt.h Outdated Show resolved Hide resolved

LICENSE Outdated Show resolved Hide resolved

isuruf added 2 commits November 9, 2021 15:00

Fix LICENSE

9579065

remove pragma once in stream_fmt.h

a953ef7

certik approved these changes Nov 9, 2021

View reviewed changes

isuruf merged commit 4b841d1 into symengine:master Nov 10, 2021

Serialization using cereal #1704

Serialization using cereal #1704

Uh oh!

Conversation

isuruf commented Nov 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

isuruf-bot commented Nov 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjodah Nov 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjodah commented Nov 7, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjodah Nov 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjodah commented Nov 8, 2020

Uh oh!

bjodah commented Nov 8, 2020

Uh oh!

isuruf commented Nov 8, 2020

Uh oh!

bjodah commented Nov 8, 2020

Uh oh!

isuruf commented Nov 8, 2020

Uh oh!

rikardn left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

isuruf commented Nov 7, 2021

Uh oh!

bjodah commented Nov 7, 2021

Uh oh!

bjodah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

isuruf commented Nov 6, 2020 •

edited

Loading

isuruf-bot commented Nov 6, 2020 •

edited

Loading

bjodah Nov 8, 2020 •

edited

Loading

bjodah Nov 8, 2020 •

edited

Loading

rikardn left a comment •

edited

Loading