Skip to content

Conversation

@BritikovKI
Copy link
Member

@BritikovKI BritikovKI commented Oct 29, 2024

Allows to create nonlin functions

Removed all of the constraints for the creation of nonlin predicates inside of the functions.
Only the assertions are checked for nonlinearity

@BritikovKI BritikovKI force-pushed the nonlin-la-preds branch 8 times, most recently from d89fbf6 to b1f40d9 Compare October 31, 2024 11:43
@BritikovKI BritikovKI marked this pull request as ready for review October 31, 2024 15:09
@aehyvari
Copy link
Member

aehyvari commented Nov 4, 2024

The code fails with this input

(set-logic QF_LIA)
(define-fun uninterp_mul ((a Int) (b Int)) Int (mod a b))
(assert (= (uninterp_mul 1 2) 0))
(check-sat)
(exit)

Here's the output:

(error "Divisor must be constant in linear logic")

(error "define-fun returns an unknown sort")

(error "Unknown symbol `uninterp_mul Int Int'")

(error "assertion returns an unknown sort")

sat

@aehyvari
Copy link
Member

aehyvari commented Nov 4, 2024

For our limited form of axioms this might actually be just enough. One thing is missing before I can put this to our production: To be sure that we're running the correct version, I'd need to have the version information from opensmt. See issue #800 .

@BritikovKI BritikovKI force-pushed the nonlin-la-preds branch 3 times, most recently from 46d12d9 to 8acd08a Compare November 4, 2024 10:25
@BritikovKI BritikovKI linked an issue Nov 4, 2024 that may be closed by this pull request
@BritikovKI BritikovKI force-pushed the nonlin-la-preds branch 11 times, most recently from 2810c10 to 32f3743 Compare November 5, 2024 13:51
@BritikovKI
Copy link
Member Author

BritikovKI commented Nov 5, 2024

Ok, I think generally PR is over

Though, we definitely need some sort of error handling, LANonLinearException produces errors with different format under different complilers rn. Should it be a different issue though?

{
assert(logic.isAtom(tr));
assert(logic.isLeq(tr));
auto [cons, sum] = logic.leqToConstantAndTerm(tr);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
auto [cons, sum] = logic.leqToConstantAndTerm(tr);
[[maybe_unused]] auto const [cons, sum] = logic.leqToConstantAndTerm(tr);

Copy link
Member

Choose a reason for hiding this comment

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

I would also change the name of the function to assertIsProperLeq with [[maybe_unused]] PTRef tr

assert(!logic.isTimes(sum) || ((logic.isNumVar(logic.getPterm(sum)[0]) && logic.isOne(logic.mkNeg(logic.getPterm(sum)[1]))) ||
(logic.isNumVar(logic.getPterm(sum)[1]) && logic.isOne(logic.mkNeg(logic.getPterm(sum)[0])))));
assert(logic.isPlus(sum) or logic.isTimesLin(sum) or logic.isMonomial(sum));
(void) cons; (void)sum;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(void) cons; (void)sum;


LVRef LASolver::getLAVar_single(PTRef expr_in) {

if (logic.isTimesNonlin(expr_in)) { throw NonLinException(logic.pp(expr_in)); }
Copy link
Member

Choose a reason for hiding this comment

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

Can we do a function for this? Like logic.checkIsNotNonlin. Then, for example, changing the error message would be necessary to update in just one place.

@Tomaqa Tomaqa self-requested a review October 24, 2025 15:24
return newSymb(fname, rsort, args, SymConf::Default);
return newSymbImpl(fname, rsort, args, SymConf::Default, false);
}
SymRef newInternalSymb(char const * fname, SRef rsort, vec<SRef> const & args, SymbolConfig const & symConfig) {
Copy link
Member

Choose a reason for hiding this comment

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

Add also the variant with the default config, as done in the case of newSymb.

// Constructs a new symbol.
SymRef newSymb(char const * fname, SRef rsort, vec<SRef> const & args, SymbolConfig const & symConfig);

SymRef newSymb(char const * fname, SRef rsort, vec<SRef> const & args, SymbolConfig const & symConfig) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
SymRef newSymb(char const * fname, SRef rsort, vec<SRef> const & args, SymbolConfig const & symConfig) {
SymRef newSymb(char const * fname, SRef rsort, vec<SRef> const & args, SymbolConfig const & symConfig = SymConf::Default) {

Comment on lines 47 to +49
SymRef newSymb(char const * fname, SRef rsort, vec<SRef> const & args) {
return newSymb(fname, rsort, args, SymConf::Default);
return newSymbImpl(fname, rsort, args, SymConf::Default, false);
}
Copy link
Member

Choose a reason for hiding this comment

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

delete

Comment on lines +42 to +43
SymRef SymStore::newSymbImpl(char const * fname, SRef rsort, vec<SRef> const & args, SymbolConfig const & symConfig,
bool isInternal) {
Copy link
Member

Choose a reason for hiding this comment

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

After I studied a bit Symbol.h, I believe we should integrate an internal flag into SymbolConfig (e.g. similarly to isInterpreted) instead of this isInternal parameter. This would be compatible with the existing code, allowing to use it also with declareFun etc. Connected to this, I think we should remove SymConf and instead allow graceful construction of SymbolConfig, possibly also adding some getters. For example, I see that noScoping implies isInterpreted (I dunno why, though).

Copy link
Member

Choose a reason for hiding this comment

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

That part of the code is very much historical, to put it nicely. noScoping would probably mean that the symbol is a reserved word. that could well be the reason why it's isInterpreted. I'm not convinced that the choices I made when introducing those fields were very informed. I encourage you to look at them with a critical eye.

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

Successfully merging this pull request may close these issues.

Opensmt rejects non-linear integer arithmetic too eagerly

5 participants