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

Wrong order in models #1214

Open
Halbaroth opened this issue Aug 22, 2024 · 3 comments
Open

Wrong order in models #1214

Halbaroth opened this issue Aug 22, 2024 · 3 comments
Labels
bug low-priority models This issue is related to model generation.

Comments

@Halbaroth
Copy link
Collaborator

It seems we do not print definitions in models in the same order of the corresponding declarations in the input file. For instance, the following input:

(set-option :produce-models true)
(set-logic ALL)
(declare-const r (_ BitVec 8))
(declare-const l (_ BitVec 8))
(declare-const m (_ BitVec 8))
(assert (bvule l r))
(assert (= m (bvudiv (bvadd l r) (_ bv2 8))))
(assert (not (and (bvule l m) (bvule m r))))
(check-sat)
(get-model)

gives

unknown
(
  (define-fun m () (_ BitVec 8) #x00)
  (define-fun r () (_ BitVec 8) #xff)
  (define-fun l () (_ BitVec 8) #x01)
) 

I believe that the appropriate fix consists in using term_cst of Dolmen instead of Id.t to store profiles. I tried to write the fix but it requires a lot of annoying workarounds because of the legacy frontend. I postpone the fix for the next release after getting rid of the legacy frontend.

@Halbaroth Halbaroth added bug models This issue is related to model generation. low-priority labels Aug 22, 2024
@Halbaroth Halbaroth modified the milestone: 2.7.0 Aug 22, 2024
@Gbury
Copy link
Collaborator

Gbury commented Aug 22, 2024

There has been some discussions about order of definitions in models, and it is still ongoing (see SMT-LIB/SMT-LIB-2#26).

For what it's worth, even though keeping the same order as the original problem is optimal in my opinion (basically, it helps with performance of checking the model), as long as the model is at least in topological order (i.e. constants are defined before being used), dolmen will be happy.

@Halbaroth
Copy link
Collaborator Author

Halbaroth commented Aug 22, 2024

Thanks for the clarification. I thought it was unspecified by the SMT-LIB standard (and it seems it is unclear according to your issue) but I believe that it is better for readability too.

@bclement-ocp
Copy link
Collaborator

bclement-ocp commented Aug 22, 2024

as long as the model is at least in topological order (i.e. constants are defined before being used), dolmen will be happy.

That is not the case currently (see embedded-array), but I'd say this is a bug, probably related to #1213.

Edit: to clarify, when I say "this is a bug" I mean that we should refer to the abstract constant (as @a2 S) in the definition of x, not to s.

Halbaroth added a commit to Halbaroth/alt-ergo that referenced this issue Aug 30, 2024
The goal of this PR is mainly to improve our API for models. Currently,
we use AE symbols as identifiers of declared constants in models.

Fix partially OCamlPro#1214
Halbaroth added a commit to Halbaroth/alt-ergo that referenced this issue Aug 30, 2024
The goal of this PR is mainly to improve our API for models. Currently,
we use AE symbols as identifiers of declared constants in models.

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

No branches or pull requests

3 participants