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

Added CI step to check proofs #68

Merged
merged 8 commits into from
Jun 13, 2023
Merged

Added CI step to check proofs #68

merged 8 commits into from
Jun 13, 2023

Conversation

ahelwer
Copy link
Collaborator

@ahelwer ahelwer commented Feb 17, 2023

I think this is worth including so existing (and future) proofs will be checked. There are a number of proofs that are commented out in the script due to the following issues:

Once these are fixed those comments can be removed. I added a step to cache the proof fingerprints which greatly speeds up checking for proofs that have already been checked.

@ahelwer
Copy link
Collaborator Author

ahelwer commented Feb 19, 2023

There is very high variance in how long it takes to check the proofs. Possible we could only check the ones that finish very quickly:

specifications/LoopInvariance/Quicksort.tla
Checked proofs in 58.1s
specifications/LoopInvariance/SumSequence.tla
Checked proofs in 308.6s
specifications/MisraReachability/ParReachProofs.tla
Checked proofs in 1.8s
specifications/MisraReachability/ReachabilityProofs.tla
Checked proofs in 11.9s
specifications/MisraReachability/ReachableProofs.tla
Checked proofs in 5.5s
specifications/TeachingConcurrency/Simple.tla
Checked proofs in 0.3s
specifications/TeachingConcurrency/SimpleRegular.tla
Checked proofs in 5.3s
specifications/TwoPhase/TLAPS.tla
Checked proofs in 0.3s
specifications/TwoPhase/TwoPhase.tla
Checked proofs in 0.3s
specifications/bcastByz/bcastByz.tla
Checked proofs in 266.6s
specifications/ewd840/EWD840_proof.tla
Checked proofs in 1.0s
specifications/ewd840/TLAPS.tla
Checked proofs in 0.3s
specifications/ewd998/AsyncTerminationDetection_proof.tla
Checked proofs in 1.3s
specifications/lamport_mutex/TLAPS.tla
Checked proofs in 0.3s
specifications/sums_even/TLAPS.tla
Checked proofs in 0.1s
specifications/sums_even/sums_even.tla
Checked proofs in 0.2s

I think they finish quickly no matter what when the fingerprints file is present, so another option is to cache the fingerprint files in between CI runs.

@muenchnerkindl
Copy link
Collaborator

Not sure if it's a good idea to check proofs as part of a CI run. As you observed, proof checking can require considerable CPU time. We could indicate modules that have proofs that can be checked fast (similar to the "small models" that TLC is run on), but I'm not sure if it's worth the effort.

@ahelwer
Copy link
Collaborator Author

ahelwer commented Feb 19, 2023

@muenchnerkindl if I could cache the proof fingerprints would that be useful?

@ahelwer ahelwer marked this pull request as ready for review June 7, 2023 14:57
@ahelwer
Copy link
Collaborator Author

ahelwer commented Jun 7, 2023

@lemmy @muenchnerkindl I think this draft PR is now good to merge if you think it's a good idea

Copy link
Member

@lemmy lemmy left a comment

Choose a reason for hiding this comment

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

Let's keep the TLAPS.tla files to prevent parse errors for those users who do not care about proofs, unless the committed TLAPS.tla modules break the CI.

.github/workflows/CI.yml Show resolved Hide resolved
Signed-off-by: Andrew Helwer <[email protected]>
@ahelwer
Copy link
Collaborator Author

ahelwer commented Jun 7, 2023

@lemmy I tested it out and they don't seem to break the CI; I think maybe they broke with the 1.4.5 TLAPS release which is why I removed them. Oh well added back!

ahelwer added 2 commits June 7, 2023 11:26
Signed-off-by: Andrew Helwer <[email protected]>
@lemmy lemmy merged commit 31f687b into tlaplus:master Jun 13, 2023
@ahelwer ahelwer deleted the checkproofs branch June 13, 2023 15:17
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