Skip to content

refactor:Serialisible concept - #2560

Draft
rprospero wants to merge 19 commits into
develop2from
serialisible_concept
Draft

refactor:Serialisible concept#2560
rprospero wants to merge 19 commits into
develop2from
serialisible_concept

Conversation

@rprospero

@rprospero rprospero commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

This PR replaces the old Serialisable class with class with a pair of concepts. This provides the following advantages

  1. The concepts can be in different namespaces. Instead of the old, confusing fromVector and toVector methods, we now have Serialisable::vector and Deserialisable::vector methods
  2. The handling of classes and raw types are now unified. Previously, we had many helper functions calling foo.serialise, which would fail if foo was a string or a double. The new serialiseOnto method works equally well with classes and raw types
  3. There's no need to inherit from a Serialisable class to produce toml. This also creates smaller classes, as none of the virtual member function pointers are needed
  4. The serialisation helper functions are now in a separate header (serialiserLibrary.h), which is often only needed in the .cpp file and not the .h file.

The fundamental theory of this library is that the overloaded serialiseOnto function serialises things and deserialiseOnto deserialises things. Making a type serialisable merely involves adding another overload to these two functions. For simplicity, I've added a templated overload that works on any type that had the old serialise and deserialise methods that we used before, so most of the code is largely unchanged.

I have the following changes I would like to perform before I pull this PR out of draft

  • Remove all direct references to toml outside of the serialiser files. This will make it easier to replace the TOML11 library in the future
  • Remove import of serialiserLibrary.h from an header files. This should prevent a larger library from many downstream files
    • src/classes/atom.h
    • src/classes/speciesIntra.h
    • src/math/history.h
    • src/nodes/parameter.h
    • src/nodes/serialisableData.h
    • src/templates/doubleKeyedMap.h

As a guide for preliminary reviews, the best files to look at will be:

  • src/base/serialiser.h is the new minimal header and establishes the new namespaces and the serialiseOnto and deserialiseOnto method signatures
  • src/base/serialiserLibrary.h contains both the actual serialisation concepts and the old helper functions
  • src/classes/speciesSite.cpp gives a good example on how the new library works (mostly the same with cleaner names).

@rprospero rprospero changed the title Serialisible concept refactor:Serialisible concept Jul 24, 2026
@rprospero
rprospero force-pushed the serialisible_concept branch from 99234da to 608a471 Compare July 24, 2026 13:58

@trisyoungs trisyoungs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Direction is good. Made a couple of comments.

Comment thread src/classes/speciesSites.cpp Outdated
Comment thread src/classes/speciesSites.cpp Outdated
Comment on lines +75 to +92
value["sites"] =
vector(sites_, [](const auto &sites) { return vector(sites, [](const auto isoWeight) { return isoWeight; }); });
target[tag] = value;
}

// Read values from a serialisable value
void SpeciesSites::deserialise(const SerialisedValue &node)
{
using namespace Deserialisable;
clear();

toMap(node, "set",
[&](const std::string &speciesName, const SerialisedValue &sites)
{
auto &set = sites_[speciesName];
toMap(sites, [&](const std::string &siteName, const SerialisedValue &population)
{ set[siteName] = population.as_floating(); });
});
map(node, "set",
[&](const std::string &speciesName, const SerialisedValue &sites)
{
auto &set = sites_[speciesName];
map(sites, [&](const std::string &siteName, const SerialisedValue &population)
{ set[siteName] = population.as_floating(); });
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I still have a problem with clarity here. I think this refactoring illustrates it pretty well - I'm supposedly serialising a vector, but deserialising a map. Admittedly this might be caused from the underlying type of sites_ being ResolvableKeyedVector, but you see my point!

@rprospero rprospero closed this Jul 28, 2026
@rprospero
rprospero force-pushed the serialisible_concept branch from d0a488a to 3c68748 Compare July 28, 2026 13:16
@rprospero rprospero reopened this Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants