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] - refactor: generalize Mul of Submodule and SMul of Ideal on Submodule to noncommutative setting #17708

Closed
wants to merge 30 commits into from

Conversation

alreadydone
Copy link
Contributor

@alreadydone alreadydone commented Oct 13, 2024

Instead of using Submodule.map₂ (which doesn't apply in the noncommutative setting), redefine the Mul and SMul to be defeq to AddSubmonoid.mul. In general, if a semiring A is a module over a semiring R such that IsScalarTower R A A is true, then the product of a Submodule R A with a Set A will be a Submodule R A, but we do not need this heterogeneous multiplication yet. We mainly intend to apply this to the case A = R.


Preparation for the Hopkins–Levitzki theorem #17908. E.g. this is necessary to state that the Jacobson radical of an Artinian ring is nilpotent.

Open in Gitpod

@alreadydone alreadydone added awaiting-CI t-algebra Algebra (groups, rings, fields, etc) labels Oct 13, 2024
Copy link

github-actions bot commented Oct 13, 2024

PR summary 3c5a448021

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

+ instance : NonUnitalSemiring (Submodule R A)
+ instance : Pow (Submodule R A) ℕ
+ instance : SMul (Ideal R) (Submodule R M)
+ instance [NoZeroDivisors R] : NoZeroDivisors (Ideal R)
+ instance {S A : Type*} [Semiring S] [SMul R S] [AddCommMonoid A] [Module R A] [Module S A]
+ mul_eq_map₂
+ npowRec_add
+ npowRec_succ
+ pow_eq_npowRec
+ pow_one
+ pow_succ
+ pow_zero
+ smul_eq_map₂
+ smul_toAddSubmonoid
- hasSMul'
- instance : Mul (Ideal R)
- instance {R : Type*} [CommSemiring R] [NoZeroDivisors R] : NoZeroDivisors (Ideal R)
- instance {R : Type*} [CommSemiring R] {S : Type*} [CommRing S] [Algebra R S]

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

The doc-module for script/declarations_diff.sh contains some details about this script.

@alreadydone alreadydone changed the title refactor: generalize mul of Submodule and smul of Ideal on Submodule to noncommutative setting refactor: generalize Mul of Submodule and SMul of Ideal on Submodule to noncommutative setting Oct 13, 2024
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Oct 14, 2024
@leanprover-community-bot-assistant leanprover-community-bot-assistant removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Oct 14, 2024
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Oct 15, 2024
@jjaassoonn
Copy link
Collaborator

I might have get the situation completely wrong.
I think now the assumption in "ideal R \bu Submodule R A" is exactly the same as "Set R \bu Submodule R A"in Submodule.pointwiseSetSMul; do we still need "ideal \bu Submodule R A" at all

@alreadydone
Copy link
Contributor Author

alreadydone commented Oct 18, 2024

I might have get the situation completely wrong. I think now the assumption in "ideal R \bu Submodule R A" is exactly the same as "Set R \bu Submodule R A"in Submodule.pointwiseSetSMul; do we still need "ideal \bu Submodule R A" at all

It's true that Ideal R \bu Submodule R A can be made a special case of Set R \bu Submodule R A under the current definition of Set R \bu Submodule R A, but I think maybe Set R \bu Submodule R A should only be defined for commutative R. Under mathematical convention, S • M consists of finite sums of products of the form s • m with s in S and m in M. When M is a R-submodule, S • M is also a R-submodule if R is commutative, but may not be in general, unless you artificially take the R-span, which is equivalent to what you do in Submodule.pointwiseSetSMul. This makes S • M (as a set) dependent on R, and deviates from mathematical convention.

My definition when specialized to the situation Ideal R \bu Ideal R is defeq to the multiplication Submodule R R * Submodule R R (also generalized to noncommutative setting in this PR). My Ideal R \bu Submodule R A factors through AddSubmonoid R \bu AddSubmonoid A and Submodule R A * Submodule R A factors through AddSubmonoid A * AddSubmonoid A, ensuring they don't rely on the R-action. This feature results in nice defeq properties (better than the original map₂ definition) and allows restrictScalars_mul etc. to be proven by rfl.

@alreadydone alreadydone added awaiting-CI and removed merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) labels Oct 18, 2024
@github-actions github-actions bot added the large-import Automatically added label for PRs with a significant increase in transitive imports label Oct 18, 2024
@jcommelin
Copy link
Member

[...] due to the lack of a typeclass that provides both left and right distributivity for SMul (unless you go all the way to Module), not all lemmas can be generalized. It would still be possible to unify Submodule Mul and Ideal-Submodule SMul, by generalizing to SMul (Submodule R A) (Submodule R M) assuming Module A M and IsScalarTower R A M. This will require moving the SMul to the same file as Mul and earlier than Mul.

Please record these observations/todos in the module docstring un ## Implementation notes and/or ## Todo.

Otherwise, LGTM

@alreadydone
Copy link
Contributor Author

alreadydone commented Oct 29, 2024

I've implemented SMul (Submodule R A) (Submodule R M) in e5fd5d6 (should I merge?), so there's no need of the TODO. I'll add a comment (implementation note?) in the module docstring about AddSubmonoid SMul lemmas.

@eric-wieser
Copy link
Member

I worry that creates diamonds, so would prefer that commit to be reviewed separately

@alreadydone
Copy link
Contributor Author

I worry that creates diamonds, so would prefer that commit to be reviewed separately

Okay, opened #18419 for that.

@alreadydone alreadydone removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Nov 7, 2024
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 🎉

bors merge

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the ready-to-merge This PR has been sent to bors. label Nov 8, 2024
mathlib-bors bot pushed a commit that referenced this pull request Nov 8, 2024
…to noncommutative setting (#17708)

Instead of using `Submodule.map₂` (which doesn't apply in the noncommutative setting), redefine the `Mul` and `SMul` to be defeq to `AddSubmonoid.mul`. In general, if a semiring A is a module over a semiring R such that `IsScalarTower R A A` is true, then the product of a `Submodule R A` with a `Set A` will be a `Submodule R A`, but we do not need this heterogeneous multiplication yet. We mainly intend to apply this to the case A = R.
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Nov 8, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title refactor: generalize Mul of Submodule and SMul of Ideal on Submodule to noncommutative setting [Merged by Bors] - refactor: generalize Mul of Submodule and SMul of Ideal on Submodule to noncommutative setting Nov 8, 2024
@mathlib-bors mathlib-bors bot closed this Nov 8, 2024
@mathlib-bors mathlib-bors bot deleted the Ideal.mul branch November 8, 2024 16:20
@alreadydone alreadydone restored the Ideal.mul branch November 8, 2024 17:05
@alreadydone alreadydone deleted the Ideal.mul branch November 8, 2024 17:27
@alreadydone alreadydone restored the Ideal.mul branch November 8, 2024 17:42
@alreadydone alreadydone deleted the Ideal.mul branch November 8, 2024 17:44
TobiasLeichtfried pushed a commit that referenced this pull request Nov 21, 2024
…to noncommutative setting (#17708)

Instead of using `Submodule.map₂` (which doesn't apply in the noncommutative setting), redefine the `Mul` and `SMul` to be defeq to `AddSubmonoid.mul`. In general, if a semiring A is a module over a semiring R such that `IsScalarTower R A A` is true, then the product of a `Submodule R A` with a `Set A` will be a `Submodule R A`, but we do not need this heterogeneous multiplication yet. We mainly intend to apply this to the case A = R.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has been sent to bors. t-algebra Algebra (groups, rings, fields, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants