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

[Merged by Bors] - feat(algebra/ordered_ring): ask for 0 < 1 earlier. #4296

Closed
wants to merge 12 commits into from

Conversation

semorrison
Copy link
Collaborator

@semorrison semorrison commented Sep 28, 2020

It used to be that linear_ordered_semiring asked for 0 < 1, while ordered_semiring didn't.

I'd prefer that ordered_semiring asks for this already:

  1. because lots of interesting examples (e.g. rings of operators) satisfy this property
  2. because the rest of mathlib doesn't care

(In fact, I think it would be good to weaken this to only ask for 0 ≤ 1, and use [nontrivial] to obtain 0 < 1 when necessary, but I don't want to try that yet.)

I know nothing about the abstract theory of ordered rings, so if someone has any reason to object to this, no problem! As an alternative I can make separate typeclasses: one that says 0 ≤ 1, and another that says the ordering is linear. However we already have far too many classes in the order hierarchy, so unless someone actually cares about ordered rings without 0 ≤ 1, let's not even admit them to the discussion!

Blocked by #4295.

@semorrison semorrison added 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 Sep 28, 2020
@github-actions github-actions bot added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Sep 28, 2020
@semorrison semorrison changed the title feat(algebra/ordered_ring): ask for 0 < 1 earlier. (deps #4295) feat(algebra/ordered_ring): ask for 0 < 1 earlier. Sep 29, 2020
@semorrison semorrison added awaiting-review The author would like community review of the PR and removed 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 Sep 29, 2020
@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Sep 29, 2020
@gebner
Copy link
Member

gebner commented Sep 29, 2020

I don't see an issue with moving the 0 < 1 to ordered semirings or making it 0 ≤ 1.

Co-authored-by: Gabriel Ebner <gebner@gebner.org>
semorrison and others added 3 commits September 30, 2020 20:59
Co-authored-by: Gabriel Ebner <gebner@gebner.org>
@robertylewis
Copy link
Member

Looks fine to me, and I also don't see any issues. @digama0 @kbuzzard any reason to object to this?

@digama0
Copy link
Member

digama0 commented Sep 30, 2020

One objection I have is that this makes unit not an ordered semiring, nor the semiring defined from a canonically ordered add_monoid with always-zero multiplication. It seems vaguely bad to me, but I don't know whether it's just like the 0 != 1 constraint in domains (which seems unnecessary for what it does but is common in traditional maths). But since we aren't really using this property in mathlib, I'm fine with going ahead with this if it's convenient for some formalization purpose; if we need the other one later we'll battle out the naming then.

@robertylewis
Copy link
Member

If there's a later refactor to weaken this to <= then unit will be an ordered semiring again.

@semorrison
Copy link
Collaborator Author

Yes, I would like to weaken this to <=, but as it seems like no one needs this immediately I'd prefer to do it in stages.

@semorrison
Copy link
Collaborator Author

On the other hand the current change actually has a mathematical significance: I have a branch with Bell's inequality and Tsirelson's inequality (about nonlocality in quantum mechanics). Typically these are either proved "in physics" or in the setting of C^*-algebras, but once ordered_ring has 0 \le 1 I can give a purely *-algebraic formulation and proof.

@gebner
Copy link
Member

gebner commented Oct 1, 2020

Ok, so we all agree that this is the way to go. We're looking forward to the 0 ≤ 1 PR.

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 Oct 1, 2020
@github-actions github-actions bot removed the awaiting-review The author would like community review of the PR label Oct 1, 2020
bors bot pushed a commit that referenced this pull request Oct 1, 2020
It used to be that `linear_ordered_semiring` asked for `0 < 1`, while `ordered_semiring` didn't.

I'd prefer that `ordered_semiring` asks for this already:
1. because lots of interesting examples (e.g. rings of operators) satisfy this property
2. because the rest of mathlib doesn't care



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Rob Lewis <Rob.y.lewis@gmail.com>
@bors
Copy link

bors bot commented Oct 1, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(algebra/ordered_ring): ask for 0 < 1 earlier. [Merged by Bors] - feat(algebra/ordered_ring): ask for 0 < 1 earlier. Oct 1, 2020
@bors bors bot closed this Oct 1, 2020
@bors bors bot deleted the ordered_semiring_zero_lt_one branch October 1, 2020 11:14
adomani pushed a commit that referenced this pull request Oct 7, 2020
It used to be that `linear_ordered_semiring` asked for `0 < 1`, while `ordered_semiring` didn't.

I'd prefer that `ordered_semiring` asks for this already:
1. because lots of interesting examples (e.g. rings of operators) satisfy this property
2. because the rest of mathlib doesn't care



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Rob Lewis <Rob.y.lewis@gmail.com>
bors bot pushed a commit that referenced this pull request Oct 19, 2020
Per [discussion](#4296 (comment)) in #4296.



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Rob Lewis <Rob.y.lewis@gmail.com>
@kbuzzard
Copy link
Member

Three weeks too late -- I don't have a problem with this. I have no opinion on whether the trivial ring is ordered.

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.

5 participants