-
Notifications
You must be signed in to change notification settings - Fork 23
Define basic well-formedness of LFunc/Factory and prove WF of Boogie Factory #317
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
base: main
Are you sure you want to change the base?
Conversation
Actually it seems running the proofs on VSCode is even faster than lake build. :/ Trying to understand what is happening behind the scene. Maybe VSCode uses async compilation by default? The timeout is for the Github Action step of leanprover/lean-action .
Strata/Languages/Boogie/Factory.lean
Outdated
| Factory.map (fun f => BoogieIdent.toPretty f.name) | ||
|
|
||
|
|
||
| set_option maxRecDepth 32768 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oof! What's going on here? Can you trace why we need this?
Also, I'd rather have set_option ... in directly above the offending construct to localize these settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is from the tactics used for proving LFuncWf.concreteEval_argmatch, but was not crystal clear. Used set_option .. in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, maybe it's grind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, but actually my impression was that it was a sequence of expensive tactics that made it eventually overflow.
If fully unrolled, the proof structure will be "take the first element of the Factory list, prove WF of the LFunc, take second element, prove WF of it, take third, ....". Since there are quite a few functions in Factory, the depth becomes quite deep but still affordable by Lean I guess.
And then here comes my theory: ideally, the expensive tactics like simp or grind are supposed to work at the "leaves" of the proof tree, avoiding multiplying the depth of the proof tree by a large constant. But my hypothesis is that this might not always the case in practice because for example discharging subgoal i might resolve hidden metavariables that will be used at subgoal i+1, making the size of the final subgoal state super large. So, the size of the sub-proof term increases as i increases, making the tactics more expensive too.
But this is indeed a conjecture. Actually, a preferred direction in the Lean world might be to define a custom computable decision procedure, prove that once the decision procedure is correct then the property holds, and use that (as the new Decidable instance in this patch did :). It did not immediately apply to this LFuncWf.concreteEval_argmatch because it is not decidable, but I am guessing that if using partial functions are allowed for Decidable then this could be resolved. But I haven't dived further, but this would be necessary if there are even more functions added to Factory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation and your theory!
Description of changes:
This patch
The motivation of this patch is somewhat in a similar context to adding conformance testing to Boogie operations. Since Boogie has many operations, there can be hidden bugs and I wanted to confirm whether these are good.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.