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

[Merged by Bors] - feat(data/tree): Add definitions about binary trees; special support for tree unit #16715

Closed
wants to merge 12 commits into from

Conversation

prakol16
Copy link
Collaborator

@prakol16 prakol16 commented Sep 29, 2022

Provide convenience API for tree unit. In particular, define a recursor for tree unit as well as other helper lemmas.

In addition, define common functions on binary trees such as nodes, num_leaves, height, and prove basic lemmas about these functions, such as the fact that the number of leaves is one more than the number of internal nodes and the height is at most the number of nodes.


Moving part of #16415 into its own PR, so that the catalan PR, which doesn't need to know anything about encodings, can proceed faster.

Open in Gitpod

@YaelDillies
Copy link
Collaborator

YaelDillies commented Sep 29, 2022

Can you use tree punit rather than defining a new inductive type? If you want simpler constructors (that is, not having to give an element of punit every time), you can still provide them as API.

@prakol16
Copy link
Collaborator Author

prakol16 commented Sep 29, 2022

Can you use tree punit rather than defining a new inductive type? If you want simpler constructors (that is, not having to give an element of punit every time), you can still provide them as API.

The reason I initially decided against it was because the use case in #16415 is to allow encodings to trees. The encoding of unit_tree should be id, whereas the polymorphic encoding of tree α based on α is probably different.

There were also minor annoyances with having some a : unit types in proofs and have to explicitly add unit_eq_star as a simp lemma every time, or call cases a where a : unit. This occurs sometimes when simp does not generate the most general form of a lemma e.g.

@[simp] def something : tree unit -> α
| tree.nil := /- something -/
| (tree.node () x y) := /- something -/

generates a simp lemma which only simplifies something (tree.node a x y) when the argument is explicitly () (rather than just a : unit). On the other hand, induction does not automatically call cases on unit types.

@YaelDillies
Copy link
Collaborator

This all sounds like things you can solve using a new recursor.

@prakol16 prakol16 added the awaiting-review The author would like community review of the PR label Oct 19, 2022
@YaelDillies YaelDillies changed the title feat(data/tree): Define unit tree, plain binary tree feat(data/tree): Special support for tree unit Oct 19, 2022
src/data/tree.lean Show resolved Hide resolved
src/data/tree.lean Outdated Show resolved Hide resolved
src/data/tree.lean Show resolved Hide resolved
src/data/tree.lean Outdated Show resolved Hide resolved
@YaelDillies YaelDillies added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Oct 20, 2022
@prakol16 prakol16 changed the title feat(data/tree): Special support for tree unit feat(data/tree): Add definitions about binary trees; special support for tree unit Oct 20, 2022
@prakol16 prakol16 added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Oct 20, 2022
Copy link
Collaborator

@YaelDillies YaelDillies left a comment

Choose a reason for hiding this comment

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

I'm fine with this PR but I would like another reviewer's opinion.

src/data/tree.lean Outdated Show resolved Hide resolved
src/data/tree.lean Outdated Show resolved Hide resolved
src/data/tree.lean Outdated Show resolved Hide resolved
src/data/tree.lean Outdated Show resolved Hide resolved
Copy link
Collaborator

@YaelDillies YaelDillies left a comment

Choose a reason for hiding this comment

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

maintainer merge

@github-actions
Copy link

🚀 Pull request has been placed on the maintainer queue by YaelDillies.

@alreadydone
Copy link
Collaborator

it's been two months so we should probably merge master first

@Vierkantor
Copy link
Collaborator

Sorry for the long wait!

bors merge

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Jan 23, 2023
bors bot pushed a commit that referenced this pull request Jan 23, 2023
…for `tree unit` (#16715)

Provide convenience API for `tree unit`. In particular, define a recursor for `tree unit` as well as other helper lemmas.

In addition, define common functions on binary trees such as `nodes`, `num_leaves`, `height`, and prove basic lemmas about these functions, such as the fact that the number of leaves is one more than the number of internal nodes and the height is at most the number of nodes.



Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
@bors
Copy link

bors bot commented Jan 23, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(data/tree): Add definitions about binary trees; special support for tree unit [Merged by Bors] - feat(data/tree): Add definitions about binary trees; special support for tree unit Jan 23, 2023
@bors bors bot closed this Jan 23, 2023
@bors bors bot deleted the unit_tree branch January 23, 2023 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants