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

fixed bogus proof in setEuclid_test.tla #98

Merged
merged 3 commits into from
Dec 21, 2023
Merged

fixed bogus proof in setEuclid_test.tla #98

merged 3 commits into from
Dec 21, 2023

Conversation

muenchnerkindl
Copy link
Contributor

I investigated in more detail the issue with the failed regression test. The failing obligation was

Cardinality (S) = 1 => S = {CHOOSE t \in S : TRUE}

During pre-processing, the old SMT backend simplified the right-hand side of this implication to

\A x : x \in S <=> (S # {} => x \in S)

which, together with the hypothesis S # {} available from the context, made the proof trivial.
In fact, the same step was proved even without appealing to CardThm, and without the assumption
of the left-hand side of the implication. This is clearly unsound.

This small PR contains a fixed proof of this test case.

NB: Running tlapm with the option --debug tempfiles still produces the input files for Isabelle
and Zenon, but no longer for SMT. More precisely, the files are created but are empty. Is this a
known issue?

@damiendoligez
Copy link
Contributor

I have a simpler fix for this proof: simply replace the proof of <3>1 on line 364 with:

        <4> Cardinality (S) = 1 => \E x \in S : S = {x}
          BY CardThm DEF Inv, TypeOK
        <4> QED
          BY Zenon

NB: Running tlapm with the option --debug tempfiles still produces the input files for Isabelle
and Zenon, but no longer for SMT. More precisely, the files are created but are empty. Is this a
known issue?

This is because of the "internal timeout" issue described in #93 (comment). I found out that the rewrite engine is not looping but generating a huge term that a later pass takes too long to process. I'm still trying to fix that, but then the result will probably be that the SMT back-end will fail with "CHOOSE not supported" instead of a timeout.

@kape1395 kape1395 mentioned this pull request Nov 13, 2023
@kape1395
Copy link
Collaborator

@muenchnerkindl , @damiendoligez , maybe we can merge either Stephan's or Damien's fix to the test case and then look for other solutions in a separate branch/PR? This failing test makes a lot of noise in other branches.

kape1395 and others added 2 commits December 21, 2023 15:51
This reverts commit 79e07b4.

Signed-off-by: Karolis Petrauskas <[email protected]>
Co-authored-by: Damien Doligez <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
@kape1395 kape1395 self-assigned this Dec 21, 2023
@kape1395 kape1395 merged commit a0ae986 into main Dec 21, 2023
13 checks passed
@kape1395 kape1395 deleted the fix-setEuclid branch December 24, 2023 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants