Skip to content
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

Introduce monitor state when creating a property #104

Merged
merged 5 commits into from
Oct 29, 2020
Merged
Show file tree
Hide file tree
Changes from 3 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
64 changes: 52 additions & 12 deletions core/prop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@
**
**/

#include "smt-switch/utils.h"

#include "core/prop.h"

#include "assert.h"
#include "core/rts.h"
#include "smt-switch/utils.h"
#include "utils/exceptions.h"
#include "utils/logger.h"

using namespace smt;

Expand All @@ -26,23 +29,14 @@ namespace pono {
Property::Property(TransitionSystem & ts, const Term & p, std::string name)
: ts_(ts), prop_(p), name_(name)
{
const UnorderedTermSet & states = ts.statevars();
TermVec free_vars;
get_free_symbolic_consts(p, free_vars);
for (auto s : free_vars) {
if (!ts.is_curr_var(s))
{
throw PonoException("Property should only use state variables");
}
}

// find a name if it wasn't provided
// if no name is associated with it in the ts, then it will just
// be the to_string of the term
if (name_.empty())
{
name_ = ts_.get_name(prop_);
}
initialize();
}

Property::Property(Property & prop, TermTranslator & tt)
Expand All @@ -58,4 +52,50 @@ Property::Property(Property & prop, TermTranslator & tt)

Property::~Property() {}

void Property::initialize()
{
if (ts_.only_curr(prop_)) {
// nothing to do
return;
}

// otherwise, need to make sure prop_ is over state vars
logger.log(1,
"Got next state or input variables in property. Generating a "
"monitor state.");

Term monitor;
size_t id = 0;
while (true) {
try {
monitor = ts_.make_statevar("_monitor_" + std::to_string(id),
ts_.make_sort(BOOL));
break;
}
catch (SmtException & e) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not very familiar with the smt-switch code: is it guaranteed that an SmtException will always be thrown when ts_.make_statevar(...) fails, regardless of the cause of failure? (to avoid getting stuck in the infinite loop)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good question -- an SmtException is the parent class for all exceptions thrown by smt-switch, but it is possible that some other type of exception is thrown in the underlying solver and is not caught correctly by smt-switch. In that case, I believe we still wouldn't be stuck in the infinite loop because we'd get an uncaught exception.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, that makes sense!

++id;
}
}
assert(monitor);

// monitor starts true
ts_.constrain_init(monitor);

if (ts_.no_next(prop_)) {
ts_.assign_next(monitor, prop_);
;
makaimann marked this conversation as resolved.
Show resolved Hide resolved
} else if (!ts_.is_functional()) {
RelationalTransitionSystem & rts =
static_cast<RelationalTransitionSystem &>(ts_);
rts.constrain_trans(rts.make_term(Equal, rts.next(monitor), prop_));
} else {
assert(ts_.is_functional());
throw PonoException(
"Cannot use next in property of a functional transition system.");
}

// update prop_
prop_ = monitor;
}

} // namespace pono
4 changes: 3 additions & 1 deletion core/prop.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,11 @@ class Property
std::string name() { return name_; };

private:
void initialize();

TransitionSystem ts_;

const smt::Term prop_;
smt::Term prop_;

std::string name_; ///< a name for the property. If no name is given, just uses the to_string

Expand Down
3 changes: 3 additions & 0 deletions engines/mbic3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ void ModelBasedIC3::initialize()
frames_.push_back({ ts_.init() });
push_frame();

assert(ts_.only_curr(bad_));
assert(ts_.only_curr(ts_.init()));

Copy link
Collaborator

Choose a reason for hiding this comment

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

A minor thing: I think that these assertions are redundant here since assert(ts_.only_curr(bad_)); is checked already in super::initialize();

Also, assert(ts_.only_curr(ts_.init())) is redundant because we check that in the TS class in the respective functions used to construct init_, but we can keep these assertions here for documentation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a very good point! I don't feel strongly either way. What do you think -- should we leave it there for documentation or remove it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then I suggest to remove them, just for consistency with the other engine classes that are derived from Prover, and which do not have these assertions.

// check if there are arrays or uninterpreted sorts and fail if so
for (auto vec : { ts_.statevars(), ts_.inputvars() }) {
for (auto st : vec) {
Expand Down
20 changes: 10 additions & 10 deletions engines/prover.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ Prover::Prover(const PonoOptions & opt,
orig_ts_(p.transition_system()),
unroller_(ts_, solver_),
options_(opt)
{
{
}

Prover::~Prover() {}
Expand All @@ -70,6 +70,7 @@ void Prover::initialize()
{
reached_k_ = -1;
bad_ = solver_->make_term(smt::PrimOp::Not, property_.prop());
assert(ts_.only_curr(bad_));
if (options_.static_coi_) {
if (!ts_.is_functional())
throw PonoException("Temporary restriction: cone-of-influence analysis currently "\
Expand Down Expand Up @@ -117,7 +118,7 @@ void Prover::print_term_dfs(const Term & term)
cout << " visiting term: " << cur << "\n";
if (cur->is_symbol())
cout << " ..is symbol\n";

for (auto child : cur) {
cout << " pushing child: " << child << "\n";
open_terms.push_back(child);
Expand Down Expand Up @@ -148,8 +149,7 @@ void Prover::print_coi_info()
cout << " " << statevar << "\n";

cout << "constraints: \n";
for (auto constr : ts_.constraints())
cout << " " << constr << "\n";
for (auto constr : ts_.constraints()) cout << " " << constr << "\n";
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this formatted by git-clang-format?

Copy link
Collaborator Author

@makaimann makaimann Oct 29, 2020

Choose a reason for hiding this comment

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

Yeah it must have been because I didn't intend to change that code otherwise, except for removing a trailing space.

}

/* Add 'term' to 'set' if it does not already appear there. */
Expand Down Expand Up @@ -196,7 +196,7 @@ void Prover::compute_term_coi(const Term & term,
collect_coi_term(new_coi_input_vars, cur);
}
}

for (auto child : cur) {
logger.log(3, " pushing child: {}", child);
open_terms.push_back(child);
Expand Down Expand Up @@ -244,9 +244,9 @@ void Prover::compute_coi_next_state_funcs()
for (auto sv : new_coi_input_vars) {
collect_coi_term(inputvars_in_coi_, sv);
}

new_coi_state_vars.clear();
new_coi_input_vars.clear();
new_coi_input_vars.clear();
}
}

Expand All @@ -263,10 +263,10 @@ void Prover::compute_coi_trans_constraints()
}

/* Add newly collected variables to global collections. */
for (auto sv : new_coi_state_vars)
for (auto sv : new_coi_state_vars)
if (statevars_in_coi_.find(sv) == statevars_in_coi_.end())
statevars_in_coi_.insert(sv);

for (auto sv : new_coi_input_vars)
if (inputvars_in_coi_.find(sv) == inputvars_in_coi_.end())
inputvars_in_coi_.insert(sv);
Expand Down Expand Up @@ -341,7 +341,7 @@ void Prover::compute_coi()
logger.log(3, " - inputvar {}", var);
}
}

bool Prover::witness(std::vector<UnorderedTermMap> & out)
{
// TODO: make sure the solver state is SAT
Expand Down
28 changes: 3 additions & 25 deletions frontends/btor2_encoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -402,31 +402,9 @@ void BTOR2Encoder::parse(const std::string filename)
no_next_states_.erase(id2statenum.at(l_->args[0]));
} else if (l_->tag == BTOR2_TAG_bad) {
Term bad = bv_to_bool(termargs_[0]);
TermVec free_vars;
get_free_symbolic_consts(bad, free_vars);
const UnorderedTermSet & states = ts_.statevars();

bool need_witness = false;
for (auto s : free_vars) {
if (states.find(s) == states.end()) {
need_witness = true;
break;
}
}

if (need_witness) {
Term witness =
ts_.make_statevar("witness_" + std::to_string(witness_id_++),
solver_->make_sort(BOOL));
ts_.constrain_init(witness);
ts_.assign_next(witness, solver_->make_term(Not, bad));
propvec_.push_back(witness);
terms_[l_->id] = witness;
} else {
Term prop = solver_->make_term(Not, bad);
propvec_.push_back(prop);
terms_[l_->id] = prop;
}
Term prop = solver_->make_term(Not, bad);
propvec_.push_back(prop);
terms_[l_->id] = prop;
} else if (l_->tag == BTOR2_TAG_justice) {
std::cout << "Warning: ignoring justice term" << std::endl;
justicevec_.push_back(termargs_[0]);
Expand Down