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

feat(algebra/algebra/basic): Algebras are bimodules #10716

Closed
wants to merge 214 commits into from

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Dec 11, 2021

This changes the definition of algebra to include a right action, such that algebra R A automatically implies a compatible left- and right- R-module structure.

The module ℕᵐᵒᵖ R and module ℤᵐᵒᵖ R instances this introduces create diamonds when R=ℕ and R=ℤ with semiring.to_opposite_module (since mul_comm is not true definitionally), but fixing those would require adding an op_nsmul field to add_comm_monoid which would make this PR huge!

The same change that is made to algebra R A also ends up being made to lie_algebra R L, although this is less in the way since lie_algebra R L already extends module R L, unlike algebra R A which does not directly extend module R A.

Most of the extra typeclass arguments that are needed in various places stem from module.End R M now only being an R-algebra if M is an R-bimodule.


This is running into an issue flagged by the fails_quickly linter, which can be reproduced as:

import algebra.algebra.restrict_scalars

variables {R S M : Type*} [comm_semiring R] [add_comm_monoid M]

-- works as expected
example [semiring S] [algebra R S] [module Sᵐᵒᵖ M] : module Rᵐᵒᵖ (restrict_scalars R S M) :=
by apply_instance

-- fails as expected
example [comm_semiring S] [algebra R S] : module Rᵐᵒᵖ (restrict_scalars R S M) := by apply_instance 

section disabled_instance
local attribute [-instance] algebra.to_opposite_module
-- fails as expected
example [semiring S] [algebra R S] : module Rᵐᵒᵖ (restrict_scalars R S M) := by apply_instance  
end disabled_instance

-- timeout, trace repeats `cached failure for comm_semiring S` forever
example [semiring S] [algebra R S] : module Rᵐᵒᵖ (restrict_scalars R S M) := by apply_instance 

Open in Gitpod

@eric-wieser eric-wieser added the RFC Request for comment label Dec 11, 2021
@github-actions github-actions bot 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 Dec 11, 2021
@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 Dec 13, 2021
@github-actions github-actions bot added the merge-conflict Please `git merge origin/master` then a bot will remove this label. label Dec 13, 2021
@eric-wieser eric-wieser added the t-algebra Algebra (groups, rings, fields etc) label Jul 26, 2022
@Vierkantor
Copy link
Collaborator

This is running into an issue flagged by the fails_quickly linter, which can be reproduced as:

What was the resolution of this issue?

@Vierkantor Vierkantor self-assigned this Sep 30, 2022
Copy link
Collaborator

@Vierkantor Vierkantor left a comment

Choose a reason for hiding this comment

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

I'm a bit worried about the loss of usability we'll get, especially because of having to insert [module Rᵐᵒᵖ M] [is_central_scalar R M] everywhere. I'd like to see a nicer way (ideally closer to standard mathematical notation), how about a new typeclass [bimodule R M]?

@@ -115,10 +118,14 @@ See the implementation notes in this file for discussion of the details of this
@[nolint has_inhabited_instance]
class algebra (R : Type u) (A : Type v) [comm_semiring R] [semiring A]
extends has_smul R A, R →+* A :=
[to_has_opposite_smul : has_smul Rᵐᵒᵖ A]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd argue that it would be more convenient, adding a field op_smul and making to_has_opposite_smul an instance.

Copy link
Member Author

Choose a reason for hiding this comment

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

The advantage of this spelling is that lean automatically picks up the instance as an instance argument if it already exists

@@ -1359,13 +1408,15 @@ section int

variables (R : Type*) [ring R]

-- FIXME
Copy link
Collaborator

Choose a reason for hiding this comment

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

What should be fixed, or has it been fixed now?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is referring to the fact that we would need a add_comm_group.rzsmul field otherwise we end up with another diamond again.

@Vierkantor Vierkantor 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 Sep 30, 2022
@eric-wieser
Copy link
Member Author

This is running into an issue flagged by the fails_quickly linter, which can be reproduced as:

What was the resolution of this issue?

It was to mess with the unfolding of some of the mul_opposite instances

bors bot pushed a commit that referenced this pull request Feb 23, 2023
…tive rings (#18384)

This changes the definition of multiplication on `triv_sq_zero_ext R M` as follows
```diff
-x * y = (x.1 * y.1, x.1 • y.2 + y.1 • x.2)
+x * y = (x.1 * y.1, x.1 • y.2 + op y.1 • x.2)
```
which for `R=M` gives the more natural `x.1 * y.2 + x.2 * y.1` in the second term instead of  `x.1 * y.2 + y.1 * x.2`.

This adds some slightly annoying typeclass arguments that could eventually be cleaned up by #10716; but that PR has rotted under the mathlib4 tide. Either way, the weird typeclass arguments needed with `triv_sq_zero_ext R M` are all invisible when working with `dual_number R`.

This is motivated by wanting to talk about `dual_number (quaternion R)` or  `dual_number (matrix n n R)`.

Unfortunately this breaks the `topological_semiring` instance (making it need an `@` and trigger the `fails_quickly` linter), but this instance isn't really interesting anyway.
@semorrison semorrison added the too-late This PR was ready too late for inclusion in mathlib3 label Jul 16, 2023
This merge was performed without any cache available, so will almost certainly fail CI.
@eric-wieser eric-wieser requested review from a team as code owners September 9, 2023 14:56
@github-actions github-actions bot added the modifies-synchronized-file This PR touches a files that has already been ported to mathlib4, and may need a synchronization PR. label Sep 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
awaiting-author A reviewer has asked the author a question or requested changes modifies-synchronized-file This PR touches a files that has already been ported to mathlib4, and may need a synchronization PR. RFC Request for comment t-algebra Algebra (groups, rings, fields etc) too-late This PR was ready too late for inclusion in mathlib3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants