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

Improve behavior of mod, int2bv, bv2nat, etc. when called on non-constant terms #801

Closed
bclement-ocp opened this issue Sep 11, 2023 · 2 comments · Fixed by #869
Closed
Assignees

Comments

@bclement-ocp
Copy link
Collaborator

This is an alternative / improved approach to (some of) #736

The crux of the issue is that if we have a function that is interpreted only on constants (i.e. a function for which we can compute, but not reason about, such as mod or bv2nat and int2bv), once we fall back to its "term" version when the argument is not a constant, we will never evaluate the result.

Simple example:

$ cat int2bv1.smt2
(set-logic ALL)

(assert (distinct ((_ int2bv 2) 0) #b00))
(check-sat)
$ alt-ergo --frontend dolmen int2bv1.smt2 2>/dev/null

; File "int2bv1.smt2", line 4, characters 1-12: Valid (0.0064) (1 steps) (goal g_1)

unsat

$ cat int2bv2.smt2
(set-logic ALL)

(define-fun x () Int 0)
(assert (distinct ((_ int2bv 2) x) #b00))
(check-sat)
$ alt-ergo --frontend dolmen int2bv2.smt2

; File "int2bv2.smt2", line 5, characters 1-12: I don't know (0.0060) (4 steps) (goal g_1)

unknown

It is not clear to me at the time of writing what the proper fix for this is. Preprocessing unconstrained equalities into hard rewrites would make sense, but we are not really equipped to do it because there are term-indexed maps everywhere. Probably the better fix would be to have a way to indicate that certain functions should trigger a second call to X.make when (one of) their argument gets substituted. I am not sure if this should be a different type of semantic values (say, Lazy or Partial) or just a flag on Term semantic values, although the latter is probably more pragmatic.

@bclement-ocp bclement-ocp self-assigned this Sep 12, 2023
@bclement-ocp
Copy link
Collaborator Author

Writing this so that I don't forget again: we can't make a second call to X.make because once the equality x = 2 is known, the value is (generally) a semantic value, not a term anymore.

@bclement-ocp
Copy link
Collaborator Author

So now I think that the best way to do this would be:

  • Add a function val compute : Symbols.t -> r list -> r. Computable symbols are symbols that can be evaluated on constants.
  • Add "computable" symbols as a separate constructor in Shostak (treated as a "builtin" theory like Ac), so that we can have the appropriate terms as leafs. This should also mean that the terms will get abstracted properly when they appear in equations.
  • When the leafs of a computable symbol get updated, call the compute function from the appropriate theory (dispatched based on is_mine_symb).

Of course, if compute is not able to compute, it should return a new computable term that we will try to evaluate again later.

It feels kind of weird because it somewhat blurs the line between terms and representatives, but I don't see a better way.

This was referenced Sep 15, 2023
bclement-ocp added a commit to bclement-ocp/alt-ergo that referenced this issue Oct 11, 2023
This patch lifts the existing implementation in `IntervalCalculus` for
delayed computation of the `pow` function (by "delayed computation" I
mean that we remember `pow` terms with uninterpreted arguments and
evaluate them once their arguments become interpreted). This is a
generic way of integrating functions that we know how to compute but not
necessarily how to reason about.

The patch is concerned about lifting the implementation for `pow` to a
generic one and adds `int2bv` and `bv2nat` as delayed functions (with
corresponding tests), but does not (yet) use it for other partially
interpreted functions from the arithmetic theory (e.g. `Modulo`,
`Real_of_int`, and the like); this will be done separately.

The implementation is placed in a new `Rel_utils` module that is
intended to provide scaffolding for generic behavior (such as delayed
computation) that can be useful for all relations. An alternative would
be to support delayed computation in a fully generic way in the
`Relation` module directly; providing the feature in `Rel_utils` is
(slightly) simpler and keeps things more decoupled; the implementation
can easily be moved to `Relation` later if we want to.

Fixes OCamlPro#801
bclement-ocp added a commit to bclement-ocp/alt-ergo that referenced this issue Oct 11, 2023
This patch lifts the existing implementation in `IntervalCalculus` for
delayed computation of the `pow` function (by "delayed computation" I
mean that we remember `pow` terms with uninterpreted arguments and
evaluate them once their arguments become interpreted). This is a
generic way of integrating functions that we know how to compute but not
necessarily how to reason about.

The patch is concerned about lifting the implementation for `pow` to a
generic one and adds `int2bv` and `bv2nat` as delayed functions (with
corresponding tests), but does not (yet) use it for other partially
interpreted functions from the arithmetic theory (e.g. `Modulo`,
`Real_of_int`, and the like); this will be done separately.

The implementation is placed in a new `Rel_utils` module that is
intended to provide scaffolding for generic behavior (such as delayed
computation) that can be useful for all relations. An alternative would
be to support delayed computation in a fully generic way in the
`Relation` module directly; providing the feature in `Rel_utils` is
(slightly) simpler and keeps things more decoupled; the implementation
can easily be moved to `Relation` later if we want to.

Fixes OCamlPro#801
bclement-ocp added a commit to bclement-ocp/alt-ergo that referenced this issue Oct 12, 2023
This patch lifts the existing implementation in `IntervalCalculus` for
delayed computation of the `pow` function (by "delayed computation" I
mean that we remember `pow` terms with uninterpreted arguments and
evaluate them once their arguments become interpreted). This is a
generic way of integrating functions that we know how to compute but not
necessarily how to reason about.

The patch is concerned about lifting the implementation for `pow` to a
generic one and adds `int2bv` and `bv2nat` as delayed functions (with
corresponding tests), but does not (yet) use it for other partially
interpreted functions from the arithmetic theory (e.g. `Modulo`,
`Real_of_int`, and the like); this will be done separately.

The implementation is placed in a new `Rel_utils` module that is
intended to provide scaffolding for generic behavior (such as delayed
computation) that can be useful for all relations. An alternative would
be to support delayed computation in a fully generic way in the
`Relation` module directly; providing the feature in `Rel_utils` is
(slightly) simpler and keeps things more decoupled; the implementation
can easily be moved to `Relation` later if we want to.

Fixes OCamlPro#801
bclement-ocp added a commit that referenced this issue Oct 12, 2023
This patch lifts the existing implementation in `IntervalCalculus` for
delayed computation of the `pow` function (by "delayed computation" I
mean that we remember `pow` terms with uninterpreted arguments and
evaluate them once their arguments become interpreted). This is a
generic way of integrating functions that we know how to compute but not
necessarily how to reason about.

The patch is concerned about lifting the implementation for `pow` to a
generic one and adds `int2bv` and `bv2nat` as delayed functions (with
corresponding tests), but does not (yet) use it for other partially
interpreted functions from the arithmetic theory (e.g. `Modulo`,
`Real_of_int`, and the like); this will be done separately.

The implementation is placed in a new `Rel_utils` module that is
intended to provide scaffolding for generic behavior (such as delayed
computation) that can be useful for all relations. An alternative would
be to support delayed computation in a fully generic way in the
`Relation` module directly; providing the feature in `Rel_utils` is
(slightly) simpler and keeps things more decoupled; the implementation
can easily be moved to `Relation` later if we want to.

Fixes #801
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 a pull request may close this issue.

1 participant