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(pfun/recursion): unbounded recursion #3778

Closed
wants to merge 35 commits into from

Conversation

cipher1024
Copy link
Collaborator

@cipher1024 cipher1024 commented Aug 14, 2020


This is a simplification of #1086. The proof automation is removed.

Todo:

  • run #list_unused_decls
  • add copyright comments
  • add module header
  • run #lint

@cipher1024 cipher1024 added WIP Work in progress blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. labels Aug 14, 2020
test/apply.lean Outdated Show resolved Hide resolved
src/data/nat/up.lean Outdated Show resolved Hide resolved
src/data/pfun/fix.lean Outdated Show resolved Hide resolved
src/data/pfun/fix.lean Outdated Show resolved Hide resolved
@digama0
Copy link
Member

digama0 commented Aug 14, 2020

Reminder: please don't force-push PRs once they have been publicly submitted.

@cipher1024
Copy link
Collaborator Author

Reminder: please don't force-push PRs once they have been publicly submitted.

Why is this an issue?

@digama0
Copy link
Member

digama0 commented Aug 14, 2020

It breaks review comments - they point to outdated and removed commits, which is very confusing for later readers.

@cipher1024
Copy link
Collaborator Author

cipher1024 commented Aug 15, 2020

There's one thing I'd like your opinion on. What do you think of using continuous and monotone bundled with the function they relate to throughout this API?

@digama0
Copy link
Member

digama0 commented Aug 15, 2020

I would say, avoid partial functions (using hypotheses in Prop as arguments to functions in Type). First, make sure you don't have any unused arguments, but if you need the argument, either totalize by returning a garbage value, or if that's not possible then package the hypothesis into the domain by bundling. I see a few definitions in this PR that deserve this treatment. pfun.fix should not require monotonicity, but you can have a pfun.fix' if you need. I see theorems with proofs in the statement of the lemma; they should use bundling or totalizing to get that out of there.

In short, I think it's a good idea, although possibly not the only solution to your problem.

@github-actions github-actions bot added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Aug 15, 2020
@bryangingechen bryangingechen changed the title feat(pfun/recursion): unbounded recursion (blocked by #3777) feat(pfun/recursion): unbounded recursion Aug 19, 2020
@bryangingechen bryangingechen removed the blocked-by-other-PR This PR depends on another PR which is still in the queue. A bot manages this label via PR comment. label Aug 19, 2020
@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Aug 22, 2020
@digama0
Copy link
Member

digama0 commented Sep 1, 2020

@cipher1024 I made some changes directly to the branch; please look over them and make sure I haven't done anything too damaging for your later work. The main highlights are:

  • nat.up has been renamed nat.upto, as well as the file it was in
  • category.Preorder was split from preorder_hom, which allowed most of the order.category.omega_CPO file to go into order.omega_CPO
  • The lawful_fix file has split from fix, separating the stuff that needs CPOs from the rest

Along with that there are a lot of small tweaks to documentation comments and proofs which shouldn't be too important.

Assuming everything is to your liking, I think we're ready to merge.

bors d+

@bors
Copy link

bors bot commented Sep 1, 2020

✌️ cipher1024 can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@bryangingechen bryangingechen added delegated The PR author may merge after reviewing final suggestions. and removed awaiting-author A reviewer has asked the author a question or requested changes labels Sep 1, 2020
@cipher1024
Copy link
Collaborator Author

cipher1024 commented Sep 1, 2020

@digama0 I like the edits. Thanks for taking the time.

@cipher1024
Copy link
Collaborator Author

bors r+

@github-actions github-actions bot added the ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) label Sep 1, 2020
bors bot pushed a commit that referenced this pull request Sep 1, 2020
Co-authored-by: Mario Carneiro <di.gama@gmail.com>
@bors
Copy link

bors bot commented Sep 1, 2020

Build failed:

@bryangingechen bryangingechen removed the ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) label Sep 1, 2020
@cipher1024
Copy link
Collaborator Author

bors r+

@github-actions github-actions bot added the ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) label Sep 2, 2020
bors bot pushed a commit that referenced this pull request Sep 2, 2020
Co-authored-by: Mario Carneiro <di.gama@gmail.com>
@bors
Copy link

bors bot commented Sep 2, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(pfun/recursion): unbounded recursion [Merged by Bors] - feat(pfun/recursion): unbounded recursion Sep 2, 2020
@bors bors bot closed this Sep 2, 2020
@bors bors bot deleted the unbounded-recursion-basic branch September 2, 2020 02:19
cipher1024 added a commit that referenced this pull request Sep 2, 2020
Co-authored-by: Mario Carneiro <di.gama@gmail.com>
PatrickMassot pushed a commit that referenced this pull request Sep 8, 2020
Co-authored-by: Mario Carneiro <di.gama@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated The PR author may merge after reviewing final suggestions. 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

5 participants