-
Notifications
You must be signed in to change notification settings - Fork 84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve support for writing checkpoints in aspif format #530
Conversation
@@ -163,6 +163,17 @@ struct IncrementalControl : Control, private Output::ASPIFOutBackend { | |||
parser.pushFile(std::string(filename), logger_); | |||
parse(); | |||
} | |||
void load_aspif(Potassco::Span<char const *> files) override { | |||
for (auto it = end(files), ib = begin(files); it != ib; --it) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: this (mostly) duplicated code is inconsistent regarding the use of begin/end. At least, I'd suggest dropping the (unnecessary) using declarations from the previous version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not compile without the using declarations. I admit, I did not check why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(The duplication will stay there for clingo series 5 but it will not stay like this in the upcoming 6.)
auto backend = UBackend{}; //Output::make_backend(out, OutputFormat::reify, false, false); | ||
switch (type & 0xFFFFFFFC) { | ||
case clingo_backend_type_e::clingo_backend_type_reify: { | ||
backend = Output::make_backend(std::move(out), Output::OutputFormat::REIFY, (type & 1) == 1, (type &2) == 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: inconsistent spacing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't even see where the inconsistent spacing is anymore. In another project I switched to 100% relying on clang-format. I should setup clang-format at some point but will leave it out of this PR.
I think this adds a lot of convenience for reusing already grounded programs. In the future this feature might also become an easy way to retract programs added with multishot solving, e.g., ctl.ground([("base", [])], checkpoint=“checkpoint1”)
ctl.ground([(“p1”, [])], checkpoint=“checkpoint2”)
ctl.solve()
ctl.restore(checkpoint=“checkpoint1”) As far as I understand the backend is basically an observer that generates the aspif file during grounding, right? Do you think in the future it might be possible to also store the nogoods learned during solving? |
Restoring the same control object to a previous state is difficult. Especially, if there shall be guarantees regarding determinism, propagators, etc. The idea here was to at least make writing and loading aspif easy.
Yes, it uses the same classes that clingo uses internally.
To some extend it is already possible to store/restore learned nogoods. There are clasp's lemma-in and lemma-out options. However, this has to be used with care. The options can only be used if the underlying pogram is exactly the same. |
If you want perfect checkpoint/restore functionality, you might want to consider https://criu.org. It should run on any linux system. |
Going to merge. In case that further refinements are necessary let's just open a new issue. |
This PR adds support to write checkpoints in aspif format, which can afterward be read again using a dedicated function.
The output is
@teiesti and @thojat, let me know what you think of this...