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

Ambiguous functions in C++ API #4637

Closed
mhbackes opened this issue Aug 13, 2020 · 3 comments
Closed

Ambiguous functions in C++ API #4637

mhbackes opened this issue Aug 13, 2020 · 3 comments

Comments

@mhbackes
Copy link

mhbackes commented Aug 13, 2020

Something I noticed is that there are some pitfalls in the Z3 C++ API if you are trying to use unsat cores with the z3::optimize.

The problem is that z3::solver::add(z3::expr const &e, char const *name) adds an assertion to the solver and tracks it with the name, while z3::optimize::add(z3::expr const &e, char const *weight) (which has the same signature) does something completely different. It adds a soft constraint with weight weight converted to a number.

My suggestion is, if possible, to separate z3::optimize functions into add() and add_soft() or assert_and_track(), just like it was done in the python API.

Anyway, thanks for the awesome job you guys did with this solver!

@NikolajBjorner
Copy link
Contributor

valid point, add_soft in opt

NikolajBjorner added a commit that referenced this issue Aug 13, 2020
Signed-off-by: Nikolaj Bjorner <[email protected]>
NikolajBjorner added a commit that referenced this issue Aug 14, 2020
* pp support for regex expressions is more-or-less standard syntax

* Regex solver updates (#4636)

* std::cout debugging statements

* comment out std::cout debugging as this is now a shared fork

* convert std::cout to TRACE statements for seq_rewriter and seq_regex

* add cases to min_length and max_length for regexes

* bug fix

* update min_length and max_length functions for REs

* initial pass on simplifying derivative normal forms by eliminating redundant predicates locally

* add seq_regex_brief trace statements

* working on debugging ref count issue

* fix ref count bug and convert trace statements to seq_regex_brief

* add compact tracing for cache hits/misses

* seq_regex fix cache hit/miss tracing and wrapper around is_nullable

* minor

* label and disable more experimental changes for testing

* minor documentation / tracing

* a few more @exp annotations

* dead state elimination skeleton code

* progress on dead state elimination

* more progress on dead state elimination

* refactor dead state class to separate self-contained state_graph class

* finish factoring state_graph to only work with unsigned values, and implement separate functionality for expr* logic

* implement get_all_derivatives, add debug tracing

* trace statements for debugging is_nullable loop bug

* fix is_nullable loop bug

* comment out local nullable change and mark experimental

* pretty printing for state_graph

* rewrite state graph to remove the fragile assumption that all edges from a state are added at a time

* start of general cycle detection check + fix some comments

* implement full cycle detection procedure

* normalize derivative conditions to form 'ele <= a'

* order derivative conditions by character code

* fix confusing names m_to and m_from

* assign increasing state IDs from 1 instead of using get_id on AST node

* remove elim_condition call in get_dall_derivatives

* use u_map instead of uint_map to avoid memory leak

* remove unnecessary call to is_ground

* debugging

* small improvements to seq_regex_brief tracing

* fix bug on evil2 example

* save work

* new propagate code

* work in progress on using same seq sort for deriv calls

* avoid re-computing derivatives: use same head var for every derivative call

* use min_length on regexes to prune search

* simple implementation of can_be_in_cycle using rank function idea

* add a disabled experimental change

* minor cleanup comments, etc.

* seq_rewriter cleanup for PR

* typo noticed by Nikolaj

* move state graph to util/state_graph

* re-add accidentally removed line

* clean up seq_regex code removing obsolete functions and comments

* a few more cleanup items

* oops, missed merge change to fix compilation

* disabled change to lift unions to the top level and treat them seperately in seq_regex solver

* added get_overapprox_regex to over-approximate regex membership constraints

* replace calls to is_epsilon with a centrally available method in seq_decl_plugin

* simplifications and modifications in get_overapprox_regex and related

* added approximation support for sequence expressions that use ite

* removed is_app check that was redundant

* tweak differences with upstream

* rewrite derivative leaves

* enable Antimorov-style derivatives via lifting unions in the solver

* TODO placeholders for outputting state graph

* change order in seq_regex propagate_in_re

* implement a more restricted form of Antimorov derivatives via a special op code to indicate lifting unions

* minor

* new Antimorov optimizations based on BDD compatibility checking

* seq regex tracing for # of derivatives

* fix get_cofactors (currently this fix is buggy)

* partially revert get_cofactors buggy change

* re-implement get_cofactors to more efficiently explore nodes in the derivative expression

* dgml generation for state graph

* fix release build

* improved dgml output

* bug fixes in dgml generation

* dot output support for state_graph and moved dgml and dot output under CASSERT

* updated tracing of what regex corresponds to what state id with /tr:state_graph

* clean up & document Antimorov derivative support

* remove op cache tracing

* remove re_rank experimental idea

* small fix

* fix Antimorov derivative (important change for the good performance)

* remove unused and unnecessary code

* implemented simpler efficient get_cofactors alternative mk_deriv_accept

* simplifications in propagate_accept, and trace unusual cases

* document the various seq_regex tracing & debugging command-line options

* fix debug build (broken tracing)

* guard eager Antimorov lifting for possible disabling

* fix bug in propagate_accept Rule 1

* disable eager version of Antimorov lifting for performance reasons

* remove some remaining obsolete comments

Co-authored-by: calebstanford-msr <[email protected]>
Co-authored-by: Margus Veanes <[email protected]>

* typo

Signed-off-by: Nikolaj Bjorner <[email protected]>

* took care of comments for related PR

* #4637

Signed-off-by: Nikolaj Bjorner <[email protected]>

* build

Signed-off-by: Nikolaj Bjorner <[email protected]>

* further PR comment fixes

* updated detection of when parenthesis can be omitted to cover empty and epsilon

* always reduce macro expansions in model evaluation #4588

Signed-off-by: Nikolaj Bjorner <[email protected]>

* fixed bug in seq_unit

* pp support for regex expressions is more-or-less standard syntax

* took care of comments for related PR

* further PR comment fixes

* updated detection of when parenthesis can be omitted to cover empty and epsilon

* fixed bug in seq_unit

Co-authored-by: Caleb Stanford <[email protected]>
Co-authored-by: calebstanford-msr <[email protected]>
Co-authored-by: Nikolaj Bjorner <[email protected]>
@mhbackes
Copy link
Author

Nice.

But would it make sense if a function void add(expr const & e, char const * p) was also added to optimize, which does exactly the same as their equivalent in solver (it calls assert_and_track()), so that both solver and optimize interfaces are consistent?

@mhbackes
Copy link
Author

Great, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants