Skip to content

propt: change interface for solving under assumptions #7976

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

Merged
merged 1 commit into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/solvers/prop/prop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,15 @@ bvt propt::new_variables(std::size_t width)

propt::resultt propt::prop_solve()
{
static const bvt empty_assumptions;
++number_of_solver_calls;
return do_prop_solve();
return do_prop_solve(empty_assumptions);
Comment on lines +41 to +43
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about just using {} as an argument here instead of the static local?

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea is that the static prevents the creation of a new object, and hence, would be a tiny bit more efficient. But that may well not be measurable.

}

propt::resultt propt::prop_solve(const bvt &assumptions)
{
++number_of_solver_calls;
return do_prop_solve(assumptions);
}

std::size_t propt::get_number_of_solver_calls() const
Expand Down
12 changes: 8 additions & 4 deletions src/solvers/prop/prop.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,11 @@ class propt
// They overload this to return false and thus avoid some optimisations
virtual bool cnf_handled_well() const { return true; }

// assumptions
virtual void set_assumptions(const bvt &) { }
virtual bool has_set_assumptions() const { return false; }
// solving with assumptions
virtual bool has_assumptions() const
{
return false;
}

// variables
virtual literalt new_variable()=0;
Expand All @@ -98,6 +100,7 @@ class propt
virtual std::string solver_text() const = 0;
enum class resultt { P_SATISFIABLE, P_UNSATISFIABLE, P_ERROR };
resultt prop_solve();
resultt prop_solve(const bvt &assumptions);

// satisfying assignment
virtual tvt l_get(literalt a) const=0;
Expand All @@ -122,7 +125,8 @@ class propt
std::size_t get_number_of_solver_calls() const;

protected:
virtual resultt do_prop_solve() = 0;
// solve under the given assumption
virtual resultt do_prop_solve(const bvt &assumptions) = 0;

// to avoid a temporary for lcnf(...)
bvt lcnf_bv;
Expand Down
8 changes: 1 addition & 7 deletions src/solvers/prop/prop_conv_solver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ decision_proceduret::resultt prop_conv_solvert::dec_solve()

log.statistics() << "Solving with " << prop.solver_text() << messaget::eom;

switch(prop.prop_solve())
switch(prop.prop_solve(assumption_stack))
{
case propt::resultt::P_SATISFIABLE:
return resultt::D_SATISFIABLE;
Expand Down Expand Up @@ -531,8 +531,6 @@ void prop_conv_solvert::push(const std::vector<exprt> &assumptions)
for(const auto &assumption : assumptions)
assumption_stack.push_back(to_literal_expr(assumption).get_literal());
context_size_stack.push_back(assumptions.size());

prop.set_assumptions(assumption_stack);
}

void prop_conv_solvert::push()
Expand All @@ -543,17 +541,13 @@ void prop_conv_solvert::push()

assumption_stack.push_back(context_literal);
context_size_stack.push_back(1);

prop.set_assumptions(assumption_stack);
}

void prop_conv_solvert::pop()
{
// We remove the context from the stack.
assumption_stack.resize(assumption_stack.size() - context_size_stack.back());
context_size_stack.pop_back();

prop.set_assumptions(assumption_stack);
}

// This method out-of-line because gcc-5.5.0-12ubuntu1 20171010 miscompiles it
Expand Down
4 changes: 2 additions & 2 deletions src/solvers/refinement/bv_refinement_loop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ bv_refinementt::bv_refinementt(const infot &info)
config_(info)
{
// check features we need
PRECONDITION(prop.has_set_assumptions());
PRECONDITION(prop.has_assumptions());
PRECONDITION(prop.has_set_to());
PRECONDITION(prop.has_is_in_conflict());
}
Expand Down Expand Up @@ -102,7 +102,7 @@ decision_proceduret::resultt bv_refinementt::prop_solve()
}

push(assumptions);
propt::resultt result=prop.prop_solve();
propt::resultt result = prop.prop_solve(assumption_stack);
pop();

// clang-format off
Expand Down
2 changes: 1 addition & 1 deletion src/solvers/sat/cnf_clause_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class cnf_clause_listt:public cnft
}

protected:
resultt do_prop_solve() override
resultt do_prop_solve(const bvt &) override
{
return resultt::P_ERROR;
}
Expand Down
2 changes: 1 addition & 1 deletion src/solvers/sat/dimacs_cnf.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class dimacs_cnf_dumpt:public cnft
}

protected:
resultt do_prop_solve() override
resultt do_prop_solve(const bvt &) override
{
return resultt::P_ERROR;
}
Expand Down
6 changes: 3 additions & 3 deletions src/solvers/sat/external_sat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ void external_satt::set_assignment(literalt, bool)
UNIMPLEMENTED;
}

void external_satt::write_cnf_file(std::string cnf_file)
void external_satt::write_cnf_file(std::string cnf_file, const bvt &assumptions)
{
log.status() << "Writing temporary CNF" << messaget::eom;
std::ofstream out(cnf_file);
Expand Down Expand Up @@ -167,7 +167,7 @@ external_satt::resultt external_satt::parse_result(std::string solver_output)
return resultt::P_ERROR;
}

external_satt::resultt external_satt::do_prop_solve()
external_satt::resultt external_satt::do_prop_solve(const bvt &assumptions)
{
// are we assuming 'false'?
if(
Expand All @@ -179,7 +179,7 @@ external_satt::resultt external_satt::do_prop_solve()

// create a temporary file
temporary_filet cnf_file("external-sat", ".cnf");
write_cnf_file(cnf_file());
write_cnf_file(cnf_file(), assumptions);
auto output = execute_solver(cnf_file());
return parse_result(output);
}
12 changes: 3 additions & 9 deletions src/solvers/sat/external_sat.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class external_satt : public cnf_clause_list_assignmentt
public:
explicit external_satt(message_handlert &message_handler, std::string cmd);

bool has_set_assumptions() const override final
bool has_assumptions() const override final
{
return true;
}
Expand All @@ -27,17 +27,11 @@ class external_satt : public cnf_clause_list_assignmentt
bool is_in_conflict(literalt) const override;
void set_assignment(literalt, bool) override;

void set_assumptions(const bvt &_assumptions) override
{
assumptions = _assumptions;
}

protected:
std::string solver_cmd;
bvt assumptions;

resultt do_prop_solve() override;
void write_cnf_file(std::string);
resultt do_prop_solve(const bvt &assumptions) override;
void write_cnf_file(std::string, const bvt &);
std::string execute_solver(std::string);
resultt parse_result(std::string);
};
Expand Down
4 changes: 3 additions & 1 deletion src/solvers/sat/pbs_dimacs_cnf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,10 @@ bool pbs_dimacs_cnft::pbs_solve()
return satisfied;
}

propt::resultt pbs_dimacs_cnft::do_prop_solve()
propt::resultt pbs_dimacs_cnft::do_prop_solve(const bvt &assumptions)
{
PRECONDITION(assumptions.empty());

std::ofstream file("temp.cnf");

write_dimacs_cnf(file);
Expand Down
2 changes: 1 addition & 1 deletion src/solvers/sat/pbs_dimacs_cnf.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class pbs_dimacs_cnft:public dimacs_cnft
}

protected:
resultt do_prop_solve() override;
resultt do_prop_solve(const bvt &assumptions) override;

std::set<int> assigned;
};
Expand Down
3 changes: 2 additions & 1 deletion src/solvers/sat/satcheck_booleforce.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,9 @@ void satcheck_booleforce_baset::lcnf(const bvt &bv)
clause_counter++;
}

propt::resultt satcheck_booleforce_baset::do_prop_solve()
propt::resultt satcheck_booleforce_baset::do_prop_solve(const bvt &assumptions)
{
PRECONDITION(assumptions.empty());
PRECONDITION(status == SAT || status == INIT);

int result=booleforce_sat();
Expand Down
2 changes: 1 addition & 1 deletion src/solvers/sat/satcheck_booleforce.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class satcheck_booleforce_baset:public cnf_solvert
void lcnf(const bvt &bv) override;

protected:
resultt do_prop_solve() override;
resultt do_prop_solve(const bvt &assumptions) override;
};

class satcheck_booleforcet:public satcheck_booleforce_baset
Expand Down
18 changes: 3 additions & 15 deletions src/solvers/sat/satcheck_cadical.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ void satcheck_cadicalt::lcnf(const bvt &bv)
clause_counter++;
}

propt::resultt satcheck_cadicalt::do_prop_solve()
propt::resultt satcheck_cadicalt::do_prop_solve(const bvt &assumptions)
{
INVARIANT(status != statust::ERROR, "there cannot be an error");

Expand All @@ -105,7 +105,8 @@ propt::resultt satcheck_cadicalt::do_prop_solve()
}

for(const auto &a : assumptions)
solver->assume(a.dimacs());
if(!a.is_true())
solver->assume(a.dimacs());

switch(solver->solve())
{
Expand Down Expand Up @@ -144,19 +145,6 @@ satcheck_cadicalt::~satcheck_cadicalt()
delete solver;
}

void satcheck_cadicalt::set_assumptions(const bvt &bv)
{
// We filter out 'true' assumptions which cause spurious results with CaDiCaL.
assumptions.clear();
for(const auto &assumption : bv)
{
if(!assumption.is_true())
{
assumptions.push_back(assumption);
}
}
}

bool satcheck_cadicalt::is_in_conflict(literalt a) const
{
return solver->failed(a.dimacs());
Expand Down
9 changes: 3 additions & 6 deletions src/solvers/sat/satcheck_cadical.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ class satcheck_cadicalt : public cnf_solvert, public hardness_collectort
void lcnf(const bvt &bv) override;
void set_assignment(literalt a, bool value) override;

void set_assumptions(const bvt &_assumptions) override;
bool has_set_assumptions() const override
bool has_assumptions() const override
{
return true;
}
Expand All @@ -43,12 +42,10 @@ class satcheck_cadicalt : public cnf_solvert, public hardness_collectort
bool is_in_conflict(literalt a) const override;

protected:
resultt do_prop_solve() override;
resultt do_prop_solve(const bvt &assumptions) override;

// NOLINTNEXTLINE(readability/identifiers)
CaDiCaL::Solver * solver;

bvt assumptions;
CaDiCaL::Solver *solver;
};

#endif // CPROVER_SOLVERS_SAT_SATCHECK_CADICAL_H
27 changes: 13 additions & 14 deletions src/solvers/sat/satcheck_glucose.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ void convert(const bvt &bv, Glucose::vec<Glucose::Lit> &dest)
}
}

void convert_assumptions(const bvt &bv, Glucose::vec<Glucose::Lit> &dest)
{
dest.capacity(bv.size());

for(const auto &literal : bv)
{
if(!literal.is_true())
dest.push(Glucose::mkLit(literal.var_no(), literal.sign()));
}
}

template<typename T>
tvt satcheck_glucose_baset<T>::l_get(literalt a) const
{
Expand Down Expand Up @@ -153,7 +164,7 @@ void satcheck_glucose_baset<T>::lcnf(const bvt &bv)
}

template <typename T>
propt::resultt satcheck_glucose_baset<T>::do_prop_solve()
propt::resultt satcheck_glucose_baset<T>::do_prop_solve(const bvt &assumptions)
{
PRECONDITION(status != statust::ERROR);

Expand Down Expand Up @@ -189,7 +200,7 @@ propt::resultt satcheck_glucose_baset<T>::do_prop_solve()
else
{
Glucose::vec<Glucose::Lit> solver_assumptions;
convert(assumptions, solver_assumptions);
convert_assumptions(assumptions, solver_assumptions);

if(solver->solve(solver_assumptions))
{
Expand Down Expand Up @@ -262,18 +273,6 @@ bool satcheck_glucose_baset<T>::is_in_conflict(literalt a) const
return false;
}

template<typename T>
void satcheck_glucose_baset<T>::set_assumptions(const bvt &bv)
{
assumptions=bv;

for(const auto &literal : assumptions)
{
INVARIANT(
!literal.is_constant(), "assumption literals must not be constant");
}
}

template class satcheck_glucose_baset<Glucose::Solver>;
template class satcheck_glucose_baset<Glucose::SimpSolver>;

Expand Down
8 changes: 2 additions & 6 deletions src/solvers/sat/satcheck_glucose.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,11 @@ class satcheck_glucose_baset : public cnf_solvert, public hardness_collectort
void lcnf(const bvt &bv) override;
void set_assignment(literalt a, bool value) override;

// extra MiniSat feature: solve with assumptions
void set_assumptions(const bvt &_assumptions) override;

// extra MiniSat feature: default branching decision
void set_polarity(literalt a, bool value);

bool is_in_conflict(literalt a) const override;
bool has_set_assumptions() const override
bool has_assumptions() const override
{
return true;
}
Expand All @@ -58,12 +55,11 @@ class satcheck_glucose_baset : public cnf_solvert, public hardness_collectort
}

protected:
resultt do_prop_solve() override;
resultt do_prop_solve(const bvt &assumptions) override;

std::unique_ptr<T> solver;

void add_variables();
bvt assumptions;
};

class satcheck_glucose_no_simplifiert:
Expand Down
Loading