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

Encode infinite sets in the arrays encoding #1802

Merged
merged 10 commits into from
Jun 22, 2022
Merged

Conversation

rodrigo7491
Copy link
Collaborator

@rodrigo7491 rodrigo7491 commented May 23, 2022

  • Tests added for any new code
  • Ran make fmt-fix (or had formatting run automatically on all files edited)
  • Documentation added for any new functionality
  • Entry added to UNRELEASED.md for any new functionality

This PR encodes infinite sets as constant SMT arrays in the arrays encoding. This fixes a problem first mentioned by @konnov when checking a version of EWD998. It is also the cause of the exception thrown by the arrays encoding mentioned in #1798.

@rodrigo7491 rodrigo7491 requested a review from konnov May 23, 2022 08:02
@rodrigo7491 rodrigo7491 self-assigned this May 23, 2022
@rodrigo7491 rodrigo7491 added bug Farrays Feature: New SMT encoding with arrays labels May 23, 2022
@rodrigo7491 rodrigo7491 added this to the Arrays Encoding milestone May 23, 2022
@codecov-commenter
Copy link

codecov-commenter commented May 23, 2022

Codecov Report

Merging #1802 (dfc6cf2) into unstable (f38eb79) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##           unstable    #1802      +/-   ##
============================================
+ Coverage     76.81%   76.82%   +0.01%     
============================================
  Files           383      383              
  Lines         11837    11843       +6     
  Branches        548      549       +1     
============================================
+ Hits           9092     9098       +6     
  Misses         2745     2745              
Impacted Files Coverage Δ
...rsyte/apalache/tla/bmcmt/smt/Z3SolverContext.scala 81.54% <100.00%> (+0.28%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f38eb79...dfc6cf2. Read the comment docs.

Copy link
Collaborator

@konnov konnov left a comment

Choose a reason for hiding this comment

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

Nice! There is definitely something about supporting functions of infinite domains in the arrays encoding. However, we should be real careful about not letting cells like that to propagate into the rules that expect bounded sets, e.g., what happens if we do DOMAN f \union { 1 }?

shonfeder
shonfeder approved these changes May 31, 2022
@thpani
Copy link
Collaborator

thpani commented Jun 9, 2022

Is this ready to be merged?

@rodrigo7491
Copy link
Collaborator Author

Not yet. I discussed it with @konnov and @Kukovec and I need to do some changes. I will make it into a draft for the moment. Thanks for bringing this up @thpani.

@rodrigo7491 rodrigo7491 marked this pull request as draft June 9, 2022 13:06
…into ro/infinite-sets-bug

� Conflicts:
�	UNRELEASED.md
�	tla-bmcmt/src/main/scala/at/forsyte/apalache/tla/bmcmt/smt/Z3SolverContext.scala
@rodrigo7491 rodrigo7491 marked this pull request as ready for review June 22, 2022 07:51
@rodrigo7491
Copy link
Collaborator Author

This PR fixes the immediate crash problem reported on #1798. Further improvements to the handling of infinite sets will be done at a later stage.

@rodrigo7491 rodrigo7491 merged commit 4eca4b0 into unstable Jun 22, 2022
@rodrigo7491 rodrigo7491 deleted the ro/infinite-sets-bug branch June 22, 2022 08:00
@apalache-bot apalache-bot mentioned this pull request Jun 27, 2022
This was referenced Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Farrays Feature: New SMT encoding with arrays
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants