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

First commit: almos every clausification rule, plus some contextual rules #1

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

Mallku2
Copy link

@Mallku2 Mallku2 commented Feb 20, 2025

No description provided.

Copy link
Collaborator

@hansjoergschurr hansjoergschurr left a comment

Choose a reason for hiding this comment

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

I only skimmed the code, but it looks very promising already. What do you think about just merging this and then improving from there?
I had some smaller comments about documentation etc.

; - conclusion Bool: Conclusion of the "subproof" step.
; return: >
; A boolean indicating if "conclusion" is of the form
; @cl not assumption, premise.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think for the SMT familiar reader it might be easier to read as an S-Expr: (@cl (not assumption) premise)

)

;;;;;;;;;;;;;;;;;;;;;;;
; Rule 4: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like this style of comment.
Maybe you could also add the state of each rule here? I.e. whether it is fully checked etc.

There is one risk: when we add a rule to the Alethe document we, don't necessarily add it to the end. So rules numbers might end up shifting around.

Copy link
Author

@Mallku2 Mallku2 Mar 10, 2025

Choose a reason for hiding this comment

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

I like this style of comment. Maybe you could also add the state of each rule here? I.e. whether it is fully checked etc.

Perfect. You mean adding the state within the heading? Like in
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
; Rule 4: false (fully checked)
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
?

Or by adding a new yaml tag to the corresponding declare-rule?

There is one risk: when we add a rule to the Alethe document we, don't necessarily add it to the end. So rules numbers might end up shifting around.

With regard to the rule number, I could remove them, if they can lead to confusion in the future. Though, are they actually helpful?

Copy link
Member

Choose a reason for hiding this comment

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

Note that you can already use the rule attribute :sorry if the rule isn't properly checked yet.

Copy link
Author

Choose a reason for hiding this comment

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

Note that you can already use the rule attribute :sorry if the rule isn't properly checked yet.

Awesome! I like the checker's answer better with this ":sorry" rules.

(($last_eq_right (and (@cl (= t1 t2)))) t2)
(($last_eq_right (and (@cl (= t1 t2)) eqs)) ($last_eq_right eqs))
; TODO: are they always clauses? (seems that it is not the
; case if the premises where just "assume"s)
Copy link
Collaborator

Choose a reason for hiding this comment

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

So this is a general question: how should the assumes be handled?
They can't be wrapped in @cl, because otherwise reference checking failed. On the other hand that introduces ugly special cases here.
Maybe we should add a helper step that transfers every assumptions into a clause?

Copy link
Author

Choose a reason for hiding this comment

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

So this is a general question: how should the assumes be handled? They can't be wrapped in @cl, because otherwise reference checking failed. On the other hand that introduces ugly special cases here. Maybe we should add a helper step that transfers every assumptions into a clause?

You are totally right. That piece of code is one of my initial attempts at mechanizing something, and it shows xD I saw that you even defined a function $to_cl to that end. I will use it.

Copy link
Author

Choose a reason for hiding this comment

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

They can't be wrapped in @cl, because otherwise reference checking failed.

Ah!, forgot to ask: what would "reference checking" mean here?

; program: context_get_substitute
; args:
; - context_get_substitute nil:
; return:
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't there be something here

Copy link
Author

Choose a reason for hiding this comment

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

Yes :) That is because I have an elisp function that introduces comments, but, from time to time I forgot to complete them. Thanks!

; program: context_get_var_subst_pairs
; args:
; - context_get_var_subst_pairs nil:
; return:
Copy link
Collaborator

Choose a reason for hiding this comment

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

dito

; args:
; - $get_free_variables nil:
; return:
(program $get_free_variables ((A Type)
Copy link
Collaborator

Choose a reason for hiding this comment

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

As Andy said this can likely be simplified by having a parameter in head position.

@Mallku2
Copy link
Author

Mallku2 commented Mar 10, 2025

I only skimmed the code, but it looks very promising already. What do you think about just merging this and then improving from there? I had some smaller comments about documentation etc.

Perfect. In the meantime, I will be fixing the code following your review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants