Skip to content

Conversation

@hannes-steffenhagen-diffblue
Copy link
Contributor

@hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue commented Mar 11, 2020

Introduces a functionality to track and collect the complexity of individual
solver queries. For SAT solvers we collect the number of clauses, literals, and
variables. The functionality matches solver queries to the SSA steps that
produced them. After running the solver, we produce a JSON report that matches
program line to GOTO instruction to SSA step to solver hardness.

At present the collection is command-line optional and only works with the
default SAT solver (minisat2). Extension to other SAT and SMT solvers should be
possible.

Supercedes #5159

  • Each commit message has a non-empty body, explaining why the change was made.
  • Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • White-space or formatting changes outside the feature-related changed lines are in commits of their own.

Introduces a functionality to track and collect the complexity of individual
solver queries. For SAT solvers we collect the number of clauses, literals, and
variables. The functionality matches solver queries to the SSA steps that
produced them. After running the solver, we produce a JSON report that matches
program line to GOTO instruction to SSA step to solver hardness.

At present the collection is command-line optional and only works with the
default SAT solver (minisat2). Extension to other SAT and SMT solvers should be
possible.

A simple regression test is included.

Note: the solver-hardness structure lives in the minisat2 and is accessible via
the interface of hardness-collector (which minisat2 inherits from). Hence to
find out if symex should collect the statistics one must try-cast `prop` to
`hardness-collector`. `prop` is part of the `solvert` but not directly
accessible from `decision-procedure`. So we also had to extend the interface
that initialises the SSA conversion (to include `prop` as well).

void convert_symex_target_equation(
symex_target_equationt &equation,
decision_proceduret &decision_procedure,
Copy link
Member

Choose a reason for hiding this comment

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

undo to keep it general


// convert SSA
equation.convert(decision_procedure);
equation.convert(property_decider.get_decision_procedure());
Copy link
Member

Choose a reason for hiding this comment

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

undo to keep it general

#include <linking/static_lifetime_init.h>

#include <solvers/decision_procedure.h>
#include <solvers/flattening/boolbv.h>
Copy link
Member

Choose a reason for hiding this comment

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

not needed


convert_symex_target_equation(
equation, property_decider.get_decision_procedure(), ui_message_handler);
convert_symex_target_equation(equation, property_decider, ui_message_handler);
Copy link
Member

Choose a reason for hiding this comment

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

undo

void convert_symex_target_equation(
symex_target_equationt &,
decision_proceduret &,
goto_symex_property_decidert &,
Copy link
Member

Choose a reason for hiding this comment

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

undo


/// Provides management of goal variables that encode properties
class goto_symex_property_decidert
class goto_symex_property_decidert : public hardness_collectort
Copy link
Member

Choose a reason for hiding this comment

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

no changes required in this class

equation_generated(false),
property_decider(options, ui_message_handler, equation, ns)
{
if(options.is_set("write-solver-stats-to"))
Copy link
Member

Choose a reason for hiding this comment

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

This should happen in solver_factoryt

{
if(options.is_set("write-solver-stats-to"))
{
with_solver_hardness(property_decider, [](solver_hardnesst &hardness) {
Copy link
Member

Choose a reason for hiding this comment

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

pass decision procedure instead of property decider:
with_solver_hardness(property_decider.get_decision_procedure()).produce_report();


#include <solvers/decision_procedure.h>
#include <solvers/hardness_collector.h>
#include <solvers/prop/prop.h>
Copy link
Member

Choose a reason for hiding this comment

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

no reference to prop in this class

#include "symex_target.h"

class decision_proceduret;
class propt;
Copy link
Member

Choose a reason for hiding this comment

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

no changes required in this file

@hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue force-pushed the feature/solver-hardness branch 3 times, most recently from 025a967 to 6ae064e Compare March 16, 2020 11:09

/// Additional reporting that may result from the underlying solver, no-op by
/// default.
virtual void report()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this is just for the hardness stats right now should this be reflected in the name of this method?

Copy link
Member

Choose a reason for hiding this comment

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

I'd say no.

@hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue marked this pull request as ready for review March 16, 2020 11:10
@hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue changed the title Feature/solver hardness Collect complexity statistics of solver queries Mar 16, 2020
@karkhaz karkhaz added the aws Bugs or features of importance to AWS CBMC users label Mar 16, 2020
^\[main.assertion.\d+\] line \d+ assertion a \+ b \< 10: FAILURE$
^VERIFICATION FAILED$
^Valid JSON File$
\"SAT_hardness\": \{(\"Clauses\": \d+|\"Variables\": \d+|\"Literals\": \d+), (\"Clauses\": \d+|\"Variables\": \d+|\"Literals\": \d+), (\"Clauses\": \d+|\"Variables\": \d+|\"Literals\": \d+)\}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as tests go this isn’t very exciting, but I couldn’t figure out a good way to put specific values here either; Any changes to symex or solvers would invalidate any test that’s checking for concrete values here.

Copy link
Member

Choose a reason for hiding this comment

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

You could check that there is something non-zero, at least.

Copy link
Member

@peterschrammel peterschrammel left a comment

Choose a reason for hiding this comment

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

Looks good now. Thanks, @hannes-steffenhagen-diffblue, for cleaning this up!


/// Additional reporting that may result from the underlying solver, no-op by
/// default.
virtual void report()
Copy link
Member

Choose a reason for hiding this comment

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

I'd say no.

^\[main.assertion.\d+\] line \d+ assertion a \+ b \< 10: FAILURE$
^VERIFICATION FAILED$
^Valid JSON File$
\"SAT_hardness\": \{(\"Clauses\": \d+|\"Variables\": \d+|\"Literals\": \d+), (\"Clauses\": \d+|\"Variables\": \d+|\"Literals\": \d+), (\"Clauses\": \d+|\"Variables\": \d+|\"Literals\": \d+)\}
Copy link
Member

Choose a reason for hiding this comment

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

You could check that there is something non-zero, at least.

Copy link
Contributor

@NlightNFotis NlightNFotis left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue merged commit 1002ac5 into diffblue:develop Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aws Bugs or features of importance to AWS CBMC users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants