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(order/height): The height of a poset #15026

Closed
wants to merge 28 commits into from

Conversation

erdOne
Copy link
Member

@erdOne erdOne commented Jun 28, 2022

@erdOne erdOne added the awaiting-review The author would like community review of the PR label Jun 28, 2022
Copy link
Collaborator

@vihdzp vihdzp left a comment

Choose a reason for hiding this comment

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

General comments + a few ramblings you can probably ignore

src/order/height.lean Outdated Show resolved Hide resolved
src/order/height.lean Outdated Show resolved Hide resolved
src/order/height.lean Outdated Show resolved Hide resolved
src/order/height.lean Outdated Show resolved Hide resolved
src/order/height.lean Outdated Show resolved Hide resolved
src/order/height.lean Outdated Show resolved Hide resolved
src/order/height.lean Outdated Show resolved Hide resolved
src/order/height.lean Outdated Show resolved Hide resolved
src/order/height.lean Outdated Show resolved Hide resolved
src/data/nat/enat.lean Outdated Show resolved Hide resolved
@YaelDillies
Copy link
Collaborator

What do you want to do with this? I've thought about the height of an order myself and came to the conclusion that we would want it to be -valued.

@erdOne
Copy link
Member Author

erdOne commented Jun 28, 2022

@YaelDillies I would like to define heights of primes, Krull dimensions of rings, codimensions of irreducible closed subsets etc. (cf This Zulip thread)

What is the reason behind the conclusion? I don't have a good way to deal with the unbounded case when it is -valued.

@vihdzp
Copy link
Collaborator

vihdzp commented Jun 28, 2022

If we want a nat-valued height, perhaps we should define it on a type with well-founded <? Relevant refactor: #15023.

@erdOne
Copy link
Member Author

erdOne commented Jun 28, 2022

I think < being well-founded does not guarantee that the global height would be finite. It only guarantees that the height of all the elements are finite.

@vihdzp
Copy link
Collaborator

vihdzp commented Jun 28, 2022

Oh of course. But the nice thing is that all elements have a finite height iff < is well-founded, so this is the most general class you could define this in.

@vihdzp
Copy link
Collaborator

vihdzp commented Jun 28, 2022

Actually, more generally, we should have a height function on is_well_founded. I'll add that as yet another to-do in the aforementioned refactor.

@YaelDillies
Copy link
Collaborator

YaelDillies commented Jun 28, 2022

Can't you just use grade_min_order ℕ α or grade_bounded_order enat α and talk about grade ℕ ⊤/grade enat ⊤? Are some of the examples above not graded? That is, can maximal chains between two elements have different lengths?

@erdOne
Copy link
Member Author

erdOne commented Jun 28, 2022

Not really. Not every topological space (or ring) is catenary.

@vihdzp
Copy link
Collaborator

vihdzp commented Jun 28, 2022

Oh wait, I totally lied to you. Well founded orders don't necessarily have a nat-valued height function: consider enat. They do have an ordinal-valued height function though. That should be worked on separately, and maybe these two APIs can be linked at some point.

@vihdzp
Copy link
Collaborator

vihdzp commented Jul 2, 2022

Expanding on that previous thought: my relevant refactor introduces an is_well_founded typeclass. We could use it to define a height function taking an [is_well_founded α r] argument, similar to our current ordinal.type and ordinal.typein (in fact, ordinal.typein is just a special case of this).

This isn't really relevant to this PR though, I'm just thinking out loud.

@jcommelin
Copy link
Member

@erdOne @vihdzp Do I understand correctly that the consensus is that this PR is basically ready to go. Connections to other concepts will be done in future work?

@erdOne
Copy link
Member Author

erdOne commented Jul 18, 2022

I think yes?

@YaelDillies
Copy link
Collaborator

I'm not sure this is the right way to do things. It certainly is a bit weird if you're interested in studying finite orders. Maybe a literature dive would help here.

src/data/nat/enat.lean Outdated Show resolved Hide resolved
@urkud urkud added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Jul 20, 2022
@YaelDillies YaelDillies removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Aug 17, 2022
@erdOne erdOne added the WIP Work in progress label Aug 17, 2022
@erdOne erdOne 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 11, 2022
@YaelDillies
Copy link
Collaborator

I could golf a bit further with your changes, so I integrated them. I'm quite happy with the current state of things although some golfing should still be possible.

@alreadydone
Copy link
Collaborator

maintainer merge

@github-actions
Copy link

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

Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@alreadydone
Copy link
Collaborator

CI fails because the implicitness of the function in pairwise_of_fn changed, apparently.

@riccardobrasca riccardobrasca added the modifies-synchronized-file This PR touches a files that has already been ported to mathlib4, and may need a synchronization PR. label Jan 27, 2023
@riccardobrasca
Copy link
Member

Please open a corresponding mathlib4 PR (even an empty one).

@YaelDillies
Copy link
Collaborator

Riccardo, it's already here: leanprover-community/mathlib4#1762

@riccardobrasca
Copy link
Member

My bad, too many teaching hours these days.

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 27, 2023
bors bot pushed a commit that referenced this pull request Jan 27, 2023
Co-authored-by: Andrew Yang <36414270+erdOne@users.noreply.github.com>
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
Co-authored-by: Junyan Xu <junyanxumath@gmail.com>
Co-authored-by: Junyan Xu <junyanxu.math@gmail.com>
@bors
Copy link

bors bot commented Jan 27, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(order/height): The height of a poset [Merged by Bors] - feat(order/height): The height of a poset Jan 27, 2023
@bors bors bot closed this Jan 27, 2023
@bors bors bot deleted the chain_length branch January 27, 2023 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modifies-synchronized-file This PR touches a files that has already been ported to mathlib4, and may need a synchronization PR. ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) t-order Order hierarchy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants