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

Update model documentation #759

Merged
merged 5 commits into from
Jul 28, 2023

Conversation

Halbaroth
Copy link
Collaborator

@Halbaroth Halbaroth commented Jul 27, 2023

This PR has to be backported on the release 2.5.0.

@Halbaroth Halbaroth added this to the 2.5.0 milestone Jul 27, 2023
Copy link
Collaborator

@bclement-ocp bclement-ocp left a comment

Choose a reason for hiding this comment

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

Some wording/organization suggestions.

Do we want to mention --model-type constraints?

Since 2.5.0, Alt-Ergo also generates models in the case it concludes on the satisfiability of
the formula.
There is two ways to activate model generation:
Alt-Ergo generates best-effort models in the case it cannot conclude the unsatisfiability of
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Alt-Ergo generates best-effort models in the case it cannot conclude the unsatisfiability of
Alt-Ergo can generate best-effort models in the case it cannot conclude the unsatisfiability of

Alt-Ergo generates best-effort models in the case it cannot conclude the unsatisfiability of
the context or the validity of a goal. By default, the model format is the SMT format.

By default the model generation is disable. There is three ways to activate it:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
By default the model generation is disable. There is three ways to activate it:
Model generation is disabled by default. There are three ways to enable it:

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should list the three ways succinctly, then expand on more details for each of them. We also should provide clear guidance on which way is recommended -- I think that's --dump-models in the native language, and --produce-models for smtlib2.


By default the model generation is disable. There is three ways to activate it:

- with the `--dump-models` option, Alt-Ergo will produce a model after each `goal` in the native language
Copy link
Collaborator

Choose a reason for hiding this comment

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

--dump-models is mostly useful with the native language because SMT-LIB users are more likely to want --produce-models to enable the standard (check-sat) + (get-model) workflow, so the example should be in the native language maybe?

Also, goal is confusing in the context of model generation because the formula is negated, we should advertise check_sat rather than goal for model generation.

)
```

- with the `--produce-models` option, you turn the model generation on. You can display
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think --produce-models and (set-option :produce-models true) should be bundled together, because they do the same thing: enable support for (get-model), so that's probably the way to explain it (turn it around from "Model generation can be enabled. Once enabled, you can use (get-model)." into "We support (get-model). To use it enable model generation with --produce-models or (set-option :produce-models true).")

Comment on lines 114 to 118
You can fine-tune the moment Alt-Ergo computes its models by using the option `--interpretation=VALUE` where VALUE can be:
* "none", to disable model generation (by default);
* "first", to output the first model found;
* "every", to compute a model before each decision;
* "last", to output the last model computed before returning 'unknown'.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure we want to document the --interpretation option, except maybe advertise --interpretation every for debugging? If we talk about it, we also should be clear that --interpretation last is what --dump-models and --produce-models do.

Can't comment on --interpretation first because I am not sure what it does...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree. Actually I hesitated to remove it.

(define-fun c () Int 0)
)
```
Note: you need to select the SAT solver Tableaux as the model generation is not
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Note: you need to select the SAT solver Tableaux as the model generation is not
*Note*: you need to select the SAT solver Tableaux as the model generation is not

Also we should make it clear that --sat-solver Tableaux is only necessary with (set-option :produce-models true), it is not needed for --produce-models or --dump-models (it is implicit currently but we should make it explicit).

the formula.
There is two ways to activate model generation:
Alt-Ergo generates best-effort models in the case it cannot conclude the unsatisfiability of
the context or the validity of a goal. By default, the model format is the SMT format.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should make it clearer that models are printed in a SMT-LIB compatible format, even if the input problem is in the native language.

Also, we should probably remove the "By default" part -- if we keep it, we should document how to use a different format, and also check that the different forma still works, because I don't think it does...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In my opinion, the SMT-LIB format would be the only one to output models. I will ask to remove the other formats for the next release or we can do it now but if there is no documentation about it, we can keep the code in the 2.5.0.

@Halbaroth
Copy link
Collaborator Author

Halbaroth commented Jul 27, 2023

Some wording/organization suggestions.

Do we want to mention --model-type constraints?

We didn't test this feature so we shouldn't mention it.

@Halbaroth Halbaroth requested a review from bclement-ocp July 27, 2023 13:51
docs/sphinx_docs/Usage/index.md Outdated Show resolved Hide resolved
docs/sphinx_docs/Usage/index.md Outdated Show resolved Hide resolved
docs/sphinx_docs/Usage/index.md Outdated Show resolved Hide resolved
docs/sphinx_docs/Usage/index.md Outdated Show resolved Hide resolved
docs/sphinx_docs/Usage/index.md Outdated Show resolved Hide resolved
docs/sphinx_docs/Usage/index.md Outdated Show resolved Hide resolved
docs/sphinx_docs/Usage/index.md Outdated Show resolved Hide resolved
Comment on lines 124 to 126
*Note*: you need to select the SAT solver Tableaux as the model generation is not
supported yet by the other SAT solvers. The options `--dump-models` and `--produce-models`
select the right SAT solver for you.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also needs --frontend dolmen (which --produce-models uses, but not --dump-models).

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is still relevant, the text should mention --frontend dolmen.

@Halbaroth Halbaroth requested a review from bclement-ocp July 27, 2023 14:49
Comment on lines 43 to 44
`(set-option :produce-models true)` and the options `--sat-solver tableaux`
and `--frontend dolmen`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
`(set-option :produce-models true)` and the options `--sat-solver tableaux`
and `--frontend dolmen`.
`(set-option :produce-models true)`. This currently requires using the options `--sat-solver tableaux`
and `--frontend dolmen`.

Comment on lines 124 to 126
*Note*: you need to select the SAT solver Tableaux as the model generation is not
supported yet by the other SAT solvers. The options `--dump-models` and `--produce-models`
select the right SAT solver for you.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is still relevant, the text should mention --frontend dolmen.

@Halbaroth Halbaroth requested a review from bclement-ocp July 28, 2023 07:47
@Halbaroth Halbaroth merged commit b83152c into OCamlPro:next Jul 28, 2023
Halbaroth added a commit to Halbaroth/alt-ergo that referenced this pull request Jul 28, 2023
* Update model documentation

* Review changes

* Review changes bis

* Review changes ter

* Clarify the dolmen option
@Halbaroth Halbaroth mentioned this pull request Jul 28, 2023
@Halbaroth Halbaroth mentioned this pull request Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants