Skip to content

Conversation

@akleeman
Copy link
Contributor

@akleeman akleeman commented Jul 2, 2018

This gives you the ability to serialize a RegressionDataset provided the FeatureType is also serializable. This can come in handy when it's costly to parse data and convert it into the required features and targets. For example, you can now you can do something like:

// Parse data and cache it.
std::vector<RegressionDataset<MyFeature>> datasets = parse_lots_of_data();
{
  std::ofstream ofs(cache_path);
  cereal::BinaryOutputArchive archive(ofs);
  archive(datasets);
}

Allowing you to read them in much faster next time,

// Read the cached datasets.
std::vector<RegressionDataset<MyFeature>> datasets;
{
  std::ifstream ifs(cache_path);
  cereal::BinaryInputArchive archive(ofs);
  archive(datasets);
}

@akleeman akleeman force-pushed the serialize_datasets branch 2 times, most recently from 1589f1a to ca742da Compare July 3, 2018 06:30
to help users identify the source of serialization issues.
*/
template <class Archive>
typename std::enable_if<valid_output_serializer<ModelFit, Archive>::value,
void>::type
Copy link

Choose a reason for hiding this comment

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

this hurts me :( I feel like we should extend the character limit on our formatter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah kinda annoying. I guess it's already a pretty difficult line to read, so doesn't make it that much worse :)

public:
SerializableDiagonalMatrix() : BaseClass(){};

SerializableDiagonalMatrix(const BaseClass &other)
Copy link

Choose a reason for hiding this comment

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

you implicitly get SerializableDiagonalMatrix(const BaseClass &&other) I think, so that's the only scenario where you don't actually need to copy.

Copy link

@kmdade kmdade left a comment

Choose a reason for hiding this comment

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

lgtm.

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