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

Finite Field support #513

Merged
merged 162 commits into from
Jul 3, 2024
Merged

Finite Field support #513

merged 162 commits into from
Jul 3, 2024

Conversation

Ovascos
Copy link
Contributor

@Ovascos Ovascos commented May 24, 2024

This PR adds the theory of finite field arithmetic (QF_FFA) to Yices. The theory is solved using mcsat, however, many auxiliary parts (SMT-LIB 2 parsing, terms, types, etc.) needed to modified.
Further, it contains some code cleanup in the NRA mcsat plugin.

@coveralls
Copy link

coveralls commented May 24, 2024

Coverage Status

coverage: 65.927% (+0.2%) from 65.726%
when pulling c7ddc21 on Ovascos:ffsat
into edc0d01 on SRI-CSL:master.

Copy link
Contributor

@ahmed-irfan ahmed-irfan left a comment

Choose a reason for hiding this comment

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

Review so far...

src/context/shared_terms.c Show resolved Hide resolved
src/frontend/yices/yices_parser.c Show resolved Hide resolved
src/include/yices_types.h Outdated Show resolved Hide resolved
src/mcsat/value.h Show resolved Hide resolved
src/model/model_eval.c Outdated Show resolved Hide resolved
src/solvers/simplex/diophantine_systems.c Show resolved Hide resolved
src/terms/polynomials.h Outdated Show resolved Hide resolved
Copy link
Contributor

@ahmed-irfan ahmed-irfan left a comment

Choose a reason for hiding this comment

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

Rest of the review.

Can you also add a test case in which the finite field is not a prime number? Yices should report an error, right?

src/frontend/smt2/smt2_lexer.c Outdated Show resolved Hide resolved
src/mcsat/ff/ff_feasible_set_db.c Outdated Show resolved Hide resolved
src/mcsat/ff/ff_plugin.c Outdated Show resolved Hide resolved
src/mcsat/ff/ff_plugin_explain.c Show resolved Hide resolved
src/mcsat/ff/ff_plugin_explain.c Outdated Show resolved Hide resolved
src/mcsat/ff/ff_plugin_explain.c Outdated Show resolved Hide resolved
src/mcsat/ff/ff_plugin_explain.c Outdated Show resolved Hide resolved
src/mcsat/ff/ff_plugin_explain.c Outdated Show resolved Hide resolved
src/mcsat/ff/ff_plugin_explain.c Show resolved Hide resolved
src/mcsat/utils/value_version_set.c Outdated Show resolved Hide resolved
@Ovascos
Copy link
Contributor Author

Ovascos commented Jul 2, 2024

All comments have be taken care of. Also, fixed some todos in the finite field printer.

Copy link
Contributor

@ahmed-irfan ahmed-irfan left a comment

Choose a reason for hiding this comment

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

LGTM.

Two things to be done in future PRs:

  • Extend Yices API
  • Update Yices documentation

@ahmed-irfan ahmed-irfan merged commit 06903da into SRI-CSL:master Jul 3, 2024
15 checks passed
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.

None yet

3 participants