-
Notifications
You must be signed in to change notification settings - Fork 66
Polymorphic arrays #786
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
Polymorphic arrays #786
Conversation
|
No changes according to regression over TPTP. |
MichaelRawson
left a comment
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.
Nice, but a few questions:
- What now happens with an array of Booleans and
ARRAY_BOOL_SELECT? Are these now pseudo-Booleans? - How does the interpretation of equality work? See comment inline.
- Don't we need to change the array theory code (if any exists) to expect 5 arguments, not 3?
- Is it worth a run on some chunk of SMT-LIB? I think arrays were already (slightly) broken, but we might now be better or worse.
| static OperatorType* getNonpolymorphicOperatorType(Interpretation i); | ||
|
|
||
| static OperatorType* getArrayOperatorType(TermList arraySort, Interpretation i); | ||
| static OperatorType* getOperatorType(Interpretation i); |
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.
Could this have a comment saying roughly what it does in various cases? Especially equality, array functions, array functions on Booleans, ...
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.
Sure, I will add a comment!
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.
This is probably still a TODO?
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 added comments for equality and the array operators, not sure if the others need explaining (and if yes, maybe I'm not the authority to add comments to those).
Yes, I think it's not worth it adding them separately, given that no one seems to be interested in array reasoning in Vampire at the moment. We can change it back when needed!
This comes from here originally, I just moved it to the common place for all interpreted stuff.
I think I did and there were only axioms regarding arrays, but let me know if you find any other place where they are used!
Probably yes, I will look the related fragments up and give it a try! |
Yeah, I couldn't find anything else from a quick look. Thanks! |
|
It was a good call to run SMTLIB.. I ran with |
|
Thanks for looking into it! Good that you caught this, and the plan sounds good to me. |
|
Is this now ready to go again once merge conflicts are fixed? |
|
Still gonna run a regression over SMTLIB to see what changed... Update: TPTP discount: TPTP otter: induction: synthesis: SMTLIB discount: |
|
With SMTLIB there is some trouble: master unsat: 54782 (259) sat: 258 (0) I'll look into it when I have some time. UPDATE: Inspecting some proofs I believe we lose problems because of the increased complexity of dealing with polymorphic symbols instead of monomorphic ones:
We should weigh now how much we want this more efficient monomorphised reasoning for arrays in particular, given the extra code complexity that it comes with. |
|
I'm not so fussed about stuff lost for "heuristic" reasons like term weights. We could probably bring these back by some strategy scheduling, and failing that by introducing a flag for ignoring sort weight if that doesn't exist already. Unnecessary FOOL reasoning is more annoying, but perhaps this is the moment to do "Boolean promotion to literals" as a preprocessing step? |
I'm not sure how this would be done, but sure, we can start thinking about it. It would make sense in the higher-order context too. |
|
Could you give an example of the unnecessary FOOL reasoning you encountered? |
|
For example, whereas before it was: (I think I copied not exactly one-to-one corresponding parts.) Not sure though how much this is actually the culprit, but it certainly complicates saturation a bit. |
|
Yuck, that looks uglier than I hoped. I was expecting to do something like and get back to something neater, but the more I think about it the less that makes sense to me. I'm certainly OK with it how it is, though. |
|
No, we don't treat such Booleans in any special way, as far as I know. |
Changed arrays to be polymorphic w.r.t. their index and inner sorts.
This allowed me to also remove the infamous
MonomorphisedInterpretationand some related dead code.