Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

[Merged by Bors] - feat(archive/100-theorems-list/73_ascending_descending_sequences): Erdős–Szekeres #3074

Closed
wants to merge 10 commits into from

Conversation

b-mehta
Copy link
Collaborator

@b-mehta b-mehta commented Jun 14, 2020

Prove the Erdős-Szekeres theorem on ascending or descending sequences


Can probably be golfed, but crossing another one off the Freek list

@bryangingechen bryangingechen added the needs-documentation This PR is missing required documentation label Jun 14, 2020
@bryangingechen bryangingechen changed the title add freek 73 feat(archive/100-theorems-list/73_ascending_descending_sequences): Erdős–Szekeres Jun 14, 2020
@robertylewis
Copy link
Member

My worry, just glancing at this, is that the proof will be hard to maintain. It's a very long proof with few intermediate statements and no comments explaining the proof sketch. Do you think you could split this up a bit more? Or at the very least add comments inside the proof so readers have some landmarks?

@b-mehta
Copy link
Collaborator Author

b-mehta commented Jun 14, 2020

I don't think it'd be possible to split the proof up, but it should be very easy to add a sketch at the top and comments during the proof, I'll do that.

@bryangingechen
Copy link
Collaborator

The Wikipedia proof you refer to uses the pigeonhole principle. Do we have a reasonable API for making arguments with it in mathlib yet? cf. #2272.

@b-mehta
Copy link
Collaborator Author

b-mehta commented Jun 14, 2020

No, but this argument uses a very basic version which is basically just the cardinality of the image of an injective function

@semorrison
Copy link
Collaborator

I don't think it'd be possible to split the proof up, but it should be very easy to add a sketch at the top and comments during the proof, I'll do that.

I think it should be straightforward to split up. Essentially every let, have and suffices you have in the proof could be a separate declaration. That said, the fact it looks straightforward to split up perhaps contradicts what Rob says about the proof having few intermediate statements, so I'm left a little uncertain how valuable it is to split up. Probably it would end up better.

@bryangingechen bryangingechen added awaiting-review The author would like community review of the PR and removed needs-documentation This PR is missing required documentation labels Jun 15, 2020
Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
@semorrison
Copy link
Collaborator

I think still to do:

  1. move injective_of_lt_imp_ne to order.basic
  2. see how @robertylewis feels about the length of the proof. I think it could be both easier to read and maintain if split across half a dozen declarations, but only slightly, and I think this is a pretty robust proof already with good use of let, have, suffices. (If I'd written it I definitely would have done it with many declarations, fwiw.)

@bryangingechen bryangingechen 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 Jun 16, 2020
@robertylewis
Copy link
Member

2. see how @robertylewis feels about the length of the proof.

The comments help a lot, thanks.

My biggest worry is the long all_goals call beginning at line 106. It's a fairly long block, without milestones, and working on multiple goals at once without closing them is a recipe for confusion.

I didn't look at it in an editor and can't judge very well from the script (which is a minor warning sign itself), but maybe it would be possible to break the all_goals script into a separate lemma?

@b-mehta
Copy link
Collaborator Author

b-mehta commented Jun 17, 2020

My biggest worry is the long all_goals call beginning at line 106. It's a fairly long block, without milestones, and working on multiple goals at once without closing them is a recipe for confusion.

This block does close the two goals it's working on - should I add a comment to make that more clear?

@robertylewis
Copy link
Member

Sure, thanks.

@b-mehta b-mehta 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 Jun 24, 2020
Copy link
Collaborator

@bryangingechen bryangingechen left a comment

Choose a reason for hiding this comment

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

Just some final cosmetic suggestions. Everything else looks good to me!

bors d+

@bors
Copy link

bors bot commented Jun 24, 2020

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

Co-authored-by: Bryan Gin-ge Chen <bryangingechen@gmail.com>
@b-mehta
Copy link
Collaborator Author

b-mehta commented Jun 24, 2020

bors r+

bors bot pushed a commit that referenced this pull request Jun 24, 2020
…dős–Szekeres (#3074)

Prove the Erdős-Szekeres theorem on ascending or descending sequences
@bors
Copy link

bors bot commented Jun 24, 2020

Build failed:

Co-authored-by: Bryan Gin-ge Chen <bryangingechen@gmail.com>
@bryangingechen
Copy link
Collaborator

Oh, the latest commit on this branch might fail to build because we haven't merged master into this branch yet. I'm betting the merge commit will build though:
bors r+

@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 Jun 25, 2020
bors bot pushed a commit that referenced this pull request Jun 25, 2020
…dős–Szekeres (#3074)

Prove the Erdős-Szekeres theorem on ascending or descending sequences
@bors
Copy link

bors bot commented Jun 25, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(archive/100-theorems-list/73_ascending_descending_sequences): Erdős–Szekeres [Merged by Bors] - feat(archive/100-theorems-list/73_ascending_descending_sequences): Erdős–Szekeres Jun 25, 2020
@bors bors bot closed this Jun 25, 2020
@bors bors bot deleted the freek-73 branch June 25, 2020 02:14
cipher1024 pushed a commit to cipher1024/mathlib that referenced this pull request Mar 15, 2022
…dős–Szekeres (leanprover-community#3074)

Prove the Erdős-Szekeres theorem on ascending or descending sequences
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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

5 participants