-
Notifications
You must be signed in to change notification settings - Fork 31
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
Changes from 3 commits
e085c26
8fafb60
04661b3
11e3c6c
79b8a37
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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())); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A minor thing: I think that these assertions are redundant here since Also, There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
// check if there are arrays or uninterpreted sorts and fail if so | ||
for (auto vec : { ts_.statevars(), ts_.inputvars() }) { | ||
for (auto st : vec) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,7 +61,7 @@ Prover::Prover(const PonoOptions & opt, | |
orig_ts_(p.transition_system()), | ||
unroller_(ts_, solver_), | ||
options_(opt) | ||
{ | ||
{ | ||
} | ||
|
||
Prover::~Prover() {} | ||
|
@@ -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 "\ | ||
|
@@ -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); | ||
|
@@ -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"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this formatted by git-clang-format? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. */ | ||
|
@@ -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); | ||
|
@@ -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(); | ||
} | ||
} | ||
|
||
|
@@ -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); | ||
|
@@ -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 | ||
|
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 am not very familiar with the
smt-switch
code: is it guaranteed that anSmtException
will always be thrown whents_.make_statevar(...)
fails, regardless of the cause of failure? (to avoid getting stuck in the infinite loop)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.
Good question -- an
SmtException
is the parent class for all exceptions thrown bysmt-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.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 see, that makes sense!