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

Terms from optimization not added to matching #1163

Closed
bclement-ocp opened this issue Jul 16, 2024 · 1 comment · Fixed by #1166
Closed

Terms from optimization not added to matching #1163

bclement-ocp opened this issue Jul 16, 2024 · 1 comment · Fixed by #1166
Assignees
Labels
bug low-priority optimization This issue is related to optimization in models.
Milestone

Comments

@bclement-ocp
Copy link
Collaborator

It looks like terms that are used only inside an optimize statement are not added to the matching environment. Consider:

; issue.smt2
(set-logic ALL)
(set-option :produce-models true)
(declare-const x Int)
(define-fun myabs ((x Int)) Int (ite (< x 0) (- x) x))
(maximize (myabs x))
(assert (< x 0))
(assert (> x (- 10000)))
; (assert (or (>= (myabs x) 0) (<= (myabs x) 0)))
(check-sat)
(get-model)
(get-objectives)

We have:

$ alt-ergo issue.smt2 2>/dev/null

unknown
(
  (define-fun x () Int (- 1))
)
(objectives 
  ((myabs x) (+ oo))
)

This is incorrect. Uncommenting the tautology (or (>= (myabs x) 0) (<= (myabs x) 0)) gives the following instead:

$ alt-ergo issue.smt2 2>/dev/null

unknown
(
  (define-fun x () Int (- 9999))
)
(objectives 
  ((myabs x) 9999)
)

Unsure whether this was always the case or if it is a regression introduced by #921.

@bclement-ocp bclement-ocp added bug optimization This issue is related to optimization in models. labels Jul 16, 2024
@bclement-ocp bclement-ocp added this to the 2.6.0 milestone Jul 16, 2024
@Halbaroth
Copy link
Collaborator

Halbaroth commented Jul 16, 2024

This patch in Theory seems to work:

  let add_objective env fn value =
    (* We have to add the term [fn] and its subterms as the MaxSMT
       syntax allows to optimize expressions that are not part of
       the current context. *)
    let expr = fn.Objective.Function.e in
    let env = add_term env ~add_in_cs:false expr in
    (* Add the term for matching. *)
    let terms = Expr.Set.add expr env.terms in
    let objectives = Objective.Model.add fn value env.objectives in
    { env with objectives; terms }

I don't think we introduce regressions while merging OptimAE as in the original PR, terms weren't added to CC(X).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug low-priority optimization This issue is related to optimization in models.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants