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(algebra/ordered_*, */sub{monoid,group,ring,semiring,field,algebra}): pullback of ordered algebraic structures under an injective map #6489

Closed
wants to merge 2 commits into from

Conversation

adomani
Copy link
Collaborator

@adomani adomani commented Mar 1, 2021

Prove that the following 14 order typeclasses can be pulled back via an injective map (function.injective.*), and use them to attach 30 new instances to sub-objects:

  • ordered_comm_monoid (and the implied ordered_add_comm_monoid)
    • submonoid.to_ordered_comm_monoid
    • submodule.to_ordered_add_comm_monoid
  • ordered_comm_group (and the implied ordered_add_comm_group)
    • subgroup.to_ordered_comm_group
    • submodule.to_ordered_add_comm_group
  • ordered_cancel_comm_monoid (and the implied ordered_cancel_add_comm_monoid)
    • submonoid.to_ordered_cancel_comm_monoid
    • submodule.to_ordered_cancel_add_comm_monoid
  • linear_ordered_cancel_comm_monoid (and the implied linear_ordered_cancel_add_comm_monoid)
    • submonoid.to_linear_ordered_cancel_comm_monoid
    • submodule.to_linear_ordered_cancel_add_comm_monoid
  • linear_ordered_comm_monoid_with_zero
    • (no suitable subobject exists for monoid_with_zero)
  • linear_ordered_comm_group (and the implied linear_ordered_add_comm_group)
    • subgroup.to_linear_ordered_comm_group
    • submodule.to_linear_ordered_add_comm_group
  • ordered_semiring
    • subsemiring.to_ordered_semiring
    • subalgebra.to_ordered_semiring
  • ordered_comm_semiring
    • subsemiring.to_ordered_comm_semiring
    • subalgebra.to_ordered_comm_semiring
  • ordered_ring
    • subring.to_ordered_ring
    • subalgebra.to_ordered_ring
  • ordered_comm_ring
    • subring.to_ordered_comm_ring
    • subalgebra.to_ordered_comm_ring
  • linear_ordered_semiring
    • subring.to_linear_ordered_semiring
    • subalgebra.to_linear_ordered_semiring
  • linear_ordered_ring
    • subring.to_linear_ordered_ring
    • subalgebra.to_linear_ordered_ring
  • linear_ordered_comm_ring
    • subring.to_linear_ordered_comm_ring
    • subalgebra.to_linear_ordered_comm_ring
  • linear_ordered_field
    • subfield.to_linear_ordered_field

Zulip:
https://leanprover.zulipchat.com/#narrow/stream/113489-new-members/topic/rings.20from.20subtype


@eric-wieser
Copy link
Member

eric-wieser commented Mar 1, 2021

It might make sense to also define the obvious subtype.ordered_semiring etc instances at the same time using these defs and subtype.coe_injective

@adomani
Copy link
Collaborator Author

adomani commented Mar 1, 2021

Eric, I may be misunderstanding, but I think that there already is a semiring deduced from an injective function: function.injective.semiring. Are you saying that I should prove that the semiring structure obtained from directly getting a semiring instance or by going through an ordered_semiring and then forgetting the order coincide?

@eric-wieser
Copy link
Member

eric-wieser commented Mar 1, 2021

I said ordered_semiring not semiring :).

I guess I still said the wrong thing though. I'm suggesting you add the thing that originally motivated this work :

instance subsemiring.ordered_semiring [ordered_semiring R] : ordered_semiring (subsemiring R) :=
subtype.coe_injective.ordered_semiring sorry sorry sorry sorry

or similar, where I suspect all the sorrys are trivial (rfl, probably with binders)

Fine to leave for a follow-up though.

@adomani
Copy link
Collaborator Author

adomani commented Mar 1, 2021

Ah, I had not understood you! And I had not realized that there was still something to do! I will take a look, thanks!

@adomani adomani changed the title feat(algebra/ordered_[monoid,ring]): pullback of ordered algebraic structures under an injective map feat(algebra/ordered_[monoid,ring], ring_theory/subsemiring): pullback of ordered algebraic structures under an injective map Mar 1, 2021
@adomani
Copy link
Collaborator Author

adomani commented Mar 1, 2021

I added the ordered_semiring instance to subsemirings of an ordered_semiring. I found comm_semiring and no_zero_divisors instances. I think that integral_domain at the moment assumes comm_ring, which could be lost passing to a subsemiring.

Eric, were you refactoring integral_domain to only assume comm_semiring?

One further instance that I can think of that is inherited by subrings, should be nontrivial.

@eric-wieser
Copy link
Member

One further instance that I can think of that is inherited by subrings, should be nontrivial.

Doesn't that already exist? I added all those recently.

Eric, were you refactoring integral_domain to only assume comm_semiring?

No, I gave up on this, as I don't really know the maths too well or whether its a good idea.

@adomani
Copy link
Collaborator Author

adomani commented Mar 1, 2021

Ok, fair enough! Besides, I do not really have a good application for "integral domains" that do not necessarily have opposites.

@adomani
Copy link
Collaborator Author

adomani commented Mar 2, 2021

Eric, I pushed a version that I think has all the instances that you mentioned.

I have not checked the nontrivial instances, since you took care of them and I am not sure that I would find them anyway!

There is also the question of whether we want the linear_ordered and canonically_ordered instances, but I am not sure that I want to continue down this path in this PR. Does this seem reasonable?

From above:

Can you do the same for submonoid.to_ordered_comm_monoid and submonoid.to_ordered_monoid? (and the to_additive versions)

Still undone.

Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

One more request - can you add function.injective.ordered_comm_group and the associated subgroup.to_ordered_comm_group instance (and their additive instances)? I suspect there's nothing new to prove for them.

@eric-wieser
Copy link
Member

eric-wieser commented Mar 2, 2021

There is also the question of whether we want the linear_ordered and canonically_ordered instances, but I am not sure that I want to continue down this path in this PR. Does this seem reasonable?

It would be good to try and pick these up too. I'm not sure that canonically_ordered is true though!

If you have a list in mind of missing instances like this, how about you write it down in a new github issue, and then we proceed without them.

@eric-wieser
Copy link
Member

How about I go add the remaining instances?

@eric-wieser
Copy link
Member

Well, I've added the missing definitions

@eric-wieser eric-wieser added the awaiting-review The author would like community review of the PR label Mar 2, 2021
@eric-wieser eric-wieser changed the title feat(algebra/ordered_[monoid,ring], ring_theory/subsemiring): pullback of ordered algebraic structures under an injective map feat(algebra/ordered_*, */sub{monoid,group,ring,semiring): pullback of ordered algebraic structures under an injective map Mar 2, 2021
@eric-wieser eric-wieser changed the title feat(algebra/ordered_*, */sub{monoid,group,ring,semiring): pullback of ordered algebraic structures under an injective map feat(algebra/ordered_*, */sub{monoid,group,ring,semiring}): pullback of ordered algebraic structures under an injective map Mar 2, 2021
@eric-wieser
Copy link
Member

And I've updated the PR body to indicate all the new instances and functions.

There are five left to add for submodule, but it would be easiest to wait until a cache is available to add those.

@adomani
Copy link
Collaborator Author

adomani commented Mar 2, 2021

Eric, thank you so much for adding more instances! I am very grateful for this, since it takes me quite some time to figure out what to do, even though I have now done it a few times!

I really like the clear description that you gave in the PR body: I am tempted to see if we can get this PR merged and start with a fresh one for the remaining instances.

As for the canonical stuff, that would indeed be more interesting, since I agree with you that it is not automatic, and it would be fun to think of the right "saturation" condition to ensure that a sub_thing of a canonically_ordered_thing is also canonically_ordered.

@adomani
Copy link
Collaborator Author

adomani commented Mar 2, 2021

Eric, thanks for the further instances! If I followed the changes correctly, I think that there is one from your list that is missing:

submodule.to_linear_ordered_add_comm_group

@eric-wieser
Copy link
Member

I ran into some issues with defeq-ness between sub and neg. Hopefully pushing a fix now...

@eric-wieser
Copy link
Member

eric-wieser commented Mar 2, 2021

In fact, that issue was present even for your original ordered_ring instance.

Elsewhere in the library, we have both function.injective.ring and function.injective.ring_sub.
Because I'm lazy, this PR adds lemmas named in the style of the former, but the safer behavior of the latter. Does this seem reasonable, @Vierkantor, or should I put _sub / _div at the end of all these names?

@adomani
Copy link
Collaborator Author

adomani commented Mar 2, 2021

Wow, the PR went from being 27 changed lines to over 300!

@eric-wieser
Copy link
Member

eric-wieser commented Mar 2, 2021

Yeah, this is a great demonstration of how dire our need for better automation is - your first 27 lines contains all the interesting proofs, and the rest are just boilerplate saying "combine the proofs you already have field-wise". We haven't even added the instances for subalgebra or subfield objects in this PR yet...

…a}): pullback of ordered algebraic structures under an injective map
@eric-wieser eric-wieser force-pushed the a_bunch_of_function.injective.instances branch from 0ca5bd5 to 8ebfeef Compare March 4, 2021 12:10
@github-actions github-actions bot 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 Mar 4, 2021
@github-actions
Copy link

github-actions bot commented Mar 4, 2021

🎉 Great news! Looks like all the dependencies have been resolved:

💡 To add or remove a dependency please update this issue/PR description.

Brought to you by Dependent Issues (:robot: ). Happy coding!

@github-actions github-actions bot removed the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Mar 4, 2021
@eric-wieser
Copy link
Member

Note that there's another build of the same commit happening here, with a 40 minutes head start: https://github.com/leanprover-community/mathlib/runs/2030608827

@jcommelin
Copy link
Member

I lost track of whether this PR is now ready for review/merging or whether there are still things to be discussed.
@adomani @eric-wieser could you please update me?

Also, I completely agree that it would be great to have better automation here. But I have no idea how to design it.

@eric-wieser
Copy link
Member

This PR is strictly smaller than it was when @Vierkantor approved it, as parts of it ended up in other PRs. It's now just waiting for the build to pass, then can be merged.

Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

bors d=@eric-wieser
bors d=@adomani

src/algebra/algebra/subalgebra.lean Show resolved Hide resolved
@bors
Copy link

bors bot commented Mar 4, 2021

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

@bors
Copy link

bors bot commented Mar 4, 2021

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

Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

linter errors

src/algebra/ordered_ring.lean Outdated Show resolved Hide resolved
src/algebra/ordered_ring.lean Outdated Show resolved Hide resolved
src/algebra/ordered_ring.lean Outdated Show resolved Hide resolved
@adomani
Copy link
Collaborator Author

adomani commented Mar 4, 2021

This is so close now...

Eric, thank you so much for all your effort and your help!

@eric-wieser
Copy link
Member

bors r+ 🎉

@jcommelin
Copy link
Member

bors merge

@bors
Copy link

bors bot commented Mar 4, 2021

Already running a review

@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 Mar 4, 2021
bors bot pushed a commit that referenced this pull request Mar 4, 2021
…a}): pullback of ordered algebraic structures under an injective map (#6489)

Prove that the following 14 order typeclasses can be pulled back via an injective map (`function.injective.*`), and use them to attach 30 new instances to sub-objects:
* `ordered_comm_monoid` (and the implied `ordered_add_comm_monoid`)
  * `submonoid.to_ordered_comm_monoid`
  * `submodule.to_ordered_add_comm_monoid`
* `ordered_comm_group` (and the implied `ordered_add_comm_group`)
  * `subgroup.to_ordered_comm_group`
  * `submodule.to_ordered_add_comm_group`
* `ordered_cancel_comm_monoid` (and the implied `ordered_cancel_add_comm_monoid`)
  * `submonoid.to_ordered_cancel_comm_monoid`
  * `submodule.to_ordered_cancel_add_comm_monoid`
* `linear_ordered_cancel_comm_monoid` (and the implied `linear_ordered_cancel_add_comm_monoid`)
  * `submonoid.to_linear_ordered_cancel_comm_monoid`
  *  `submodule.to_linear_ordered_cancel_add_comm_monoid`
* `linear_ordered_comm_monoid_with_zero`
  * (no suitable subobject exists for monoid_with_zero)
* `linear_ordered_comm_group` (and the implied `linear_ordered_add_comm_group`)
  * `subgroup.to_linear_ordered_comm_group`
  * `submodule.to_linear_ordered_add_comm_group`
* `ordered_semiring`
  * `subsemiring.to_ordered_semiring`
  * `subalgebra.to_ordered_semiring`
* `ordered_comm_semiring`
  * `subsemiring.to_ordered_comm_semiring`
  * `subalgebra.to_ordered_comm_semiring`
* `ordered_ring`
  * `subring.to_ordered_ring`
  * `subalgebra.to_ordered_ring`
* `ordered_comm_ring`
  * `subring.to_ordered_comm_ring`
  * `subalgebra.to_ordered_comm_ring`
* `linear_ordered_semiring`
  * `subring.to_linear_ordered_semiring`
  * `subalgebra.to_linear_ordered_semiring`
* `linear_ordered_ring`
  * `subring.to_linear_ordered_ring`
  * `subalgebra.to_linear_ordered_ring`
* `linear_ordered_comm_ring`
  * `subring.to_linear_ordered_comm_ring`
  * `subalgebra.to_linear_ordered_comm_ring`
* `linear_ordered_field`
  * `subfield.to_linear_ordered_field`

Zulip:
https://leanprover.zulipchat.com/#narrow/stream/113489-new-members/topic/rings.20from.20subtype



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

bors bot commented Mar 5, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(algebra/ordered_*, */sub{monoid,group,ring,semiring,field,algebra}): pullback of ordered algebraic structures under an injective map [Merged by Bors] - feat(algebra/ordered_*, */sub{monoid,group,ring,semiring,field,algebra}): pullback of ordered algebraic structures under an injective map Mar 5, 2021
@bors bors bot closed this Mar 5, 2021
@bors bors bot deleted the a_bunch_of_function.injective.instances branch March 5, 2021 00:23
b-mehta pushed a commit that referenced this pull request Apr 2, 2021
…ects (#6509)

This changes `subfield.to_field` to ensure that division is defeq.

It also removes `subring.subset_comm_ring` which was identical to `subring.to_comm_ring`, renames some `subalgebra` instances to match those of `subring`s, and cleans up a few related proofs that relied on the old names.

These are cleanups split from #6489, which failed CI but was otherwise approved
b-mehta pushed a commit that referenced this pull request Apr 2, 2021
…a}): pullback of ordered algebraic structures under an injective map (#6489)

Prove that the following 14 order typeclasses can be pulled back via an injective map (`function.injective.*`), and use them to attach 30 new instances to sub-objects:
* `ordered_comm_monoid` (and the implied `ordered_add_comm_monoid`)
  * `submonoid.to_ordered_comm_monoid`
  * `submodule.to_ordered_add_comm_monoid`
* `ordered_comm_group` (and the implied `ordered_add_comm_group`)
  * `subgroup.to_ordered_comm_group`
  * `submodule.to_ordered_add_comm_group`
* `ordered_cancel_comm_monoid` (and the implied `ordered_cancel_add_comm_monoid`)
  * `submonoid.to_ordered_cancel_comm_monoid`
  * `submodule.to_ordered_cancel_add_comm_monoid`
* `linear_ordered_cancel_comm_monoid` (and the implied `linear_ordered_cancel_add_comm_monoid`)
  * `submonoid.to_linear_ordered_cancel_comm_monoid`
  *  `submodule.to_linear_ordered_cancel_add_comm_monoid`
* `linear_ordered_comm_monoid_with_zero`
  * (no suitable subobject exists for monoid_with_zero)
* `linear_ordered_comm_group` (and the implied `linear_ordered_add_comm_group`)
  * `subgroup.to_linear_ordered_comm_group`
  * `submodule.to_linear_ordered_add_comm_group`
* `ordered_semiring`
  * `subsemiring.to_ordered_semiring`
  * `subalgebra.to_ordered_semiring`
* `ordered_comm_semiring`
  * `subsemiring.to_ordered_comm_semiring`
  * `subalgebra.to_ordered_comm_semiring`
* `ordered_ring`
  * `subring.to_ordered_ring`
  * `subalgebra.to_ordered_ring`
* `ordered_comm_ring`
  * `subring.to_ordered_comm_ring`
  * `subalgebra.to_ordered_comm_ring`
* `linear_ordered_semiring`
  * `subring.to_linear_ordered_semiring`
  * `subalgebra.to_linear_ordered_semiring`
* `linear_ordered_ring`
  * `subring.to_linear_ordered_ring`
  * `subalgebra.to_linear_ordered_ring`
* `linear_ordered_comm_ring`
  * `subring.to_linear_ordered_comm_ring`
  * `subalgebra.to_linear_ordered_comm_ring`
* `linear_ordered_field`
  * `subfield.to_linear_ordered_field`

Zulip:
https://leanprover.zulipchat.com/#narrow/stream/113489-new-members/topic/rings.20from.20subtype



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
mathlib-bors bot pushed a commit to leanprover-community/mathlib4 that referenced this pull request Feb 25, 2024
…10900)

Moving these to separate files should make typeclass synthesis less expensive. Additionally two of them are quite long and this shrinks them slightly.

This handles:
* `Submonoid`
* `Subgroup`
* `Subsemiring`
* `Subring`
* `Subfield`
* `Submodule`
* `Subalgebra`

This also moves `Units.posSubgroup` into its own file.

The copyright headers are from:
* leanprover-community/mathlib#6489
* leanprover-community/mathlib#8466



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
riccardobrasca pushed a commit to leanprover-community/mathlib4 that referenced this pull request Mar 1, 2024
…10900)

Moving these to separate files should make typeclass synthesis less expensive. Additionally two of them are quite long and this shrinks them slightly.

This handles:
* `Submonoid`
* `Subgroup`
* `Subsemiring`
* `Subring`
* `Subfield`
* `Submodule`
* `Subalgebra`

This also moves `Units.posSubgroup` into its own file.

The copyright headers are from:
* leanprover-community/mathlib#6489
* leanprover-community/mathlib#8466



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
dagurtomas pushed a commit to leanprover-community/mathlib4 that referenced this pull request Mar 22, 2024
…10900)

Moving these to separate files should make typeclass synthesis less expensive. Additionally two of them are quite long and this shrinks them slightly.

This handles:
* `Submonoid`
* `Subgroup`
* `Subsemiring`
* `Subring`
* `Subfield`
* `Submodule`
* `Subalgebra`

This also moves `Units.posSubgroup` into its own file.

The copyright headers are from:
* leanprover-community/mathlib#6489
* leanprover-community/mathlib#8466



Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Louddy pushed a commit to leanprover-community/mathlib4 that referenced this pull request Apr 15, 2024
…10900)

Moving these to separate files should make typeclass synthesis less expensive. Additionally two of them are quite long and this shrinks them slightly.

This handles:
* `Submonoid`
* `Subgroup`
* `Subsemiring`
* `Subring`
* `Subfield`
* `Submodule`
* `Subalgebra`

This also moves `Units.posSubgroup` into its own file.

The copyright headers are from:
* leanprover-community/mathlib#6489
* leanprover-community/mathlib#8466



Co-authored-by: Eric Wieser <wieser.eric@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

4 participants