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

refactor(algebra/star/*): Allow for star operation on non-associative algebras#17949

Closed
mans0954 wants to merge 36 commits intomasterfrom
non-assoc-star-mul
Closed

refactor(algebra/star/*): Allow for star operation on non-associative algebras#17949
mans0954 wants to merge 36 commits intomasterfrom
non-assoc-star-mul

Conversation

@mans0954
Copy link
Copy Markdown
Collaborator

@mans0954 mans0954 commented Dec 14, 2022

Typically a * operation on a mathematical structure R equipped with a multiplication is an involutive anti-automorphism i.e.

∀ r s : R, star (r * s) = star s * star r

Currently mathlib defines a class star_semigroup to be a semigroup satisfying this property. However, the requirement for the multiplication to be associative is unnecessarily restrictive. There are important classes of star-algebra which are not associative (e.g. JB*-algebras).

This PR removes the requirement for a star_semigroup to be a semigroup, merely requiring it to have a multiplication.

I've changed the name from star_semigroup to star_magma since it's no longer a semigroup.

Zulip discussion


Open in Gitpod

Comment thread src/algebra/star/pi.lean Outdated
Comment thread src/algebra/star/self_adjoint.lean Outdated
@YaelDillies YaelDillies changed the title refactor(algebra.star): Allow for star operation on non-associative algebras refactor(algebra/star/*): Allow for star operation on non-associative algebras Dec 15, 2022
Comment thread src/algebra/star/pi.lean Outdated
Comment thread src/algebra/star/pi.lean
Comment thread src/algebra/star/pi.lean Outdated
@mans0954 mans0954 marked this pull request as ready for review January 8, 2023 04:18
@mans0954 mans0954 added the awaiting-review The author would like community review of the PR label Jan 8, 2023
Comment thread src/algebra/star/prod.lean Outdated
Comment thread src/data/matrix/basic.lean Outdated
Comment thread src/topology/continuous_function/algebra.lean Outdated
@YaelDillies YaelDillies 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 18, 2023
Comment thread src/algebra/star/basic.lean Outdated
so `star (r * s) = star s * star r`.
-/
class star_semigroup (R : Type u) [semigroup R] extends has_involutive_star R :=
class star_magma (R : Type u) [has_mul R] extends has_involutive_star R :=
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We never really use magma as a word anywhere. Maybe this is better?

Suggested change
class star_magma (R : Type u) [has_mul R] extends has_involutive_star R :=
class star_mul (R : Type u) [has_mul R] extends has_involutive_star R :=

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Okay, done.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@YaelDillies Having a field with the same name as the class seems to be a source of confusion:

Error: /home/lean/actions-runner/_work/mathlib/mathlib/src/algebra/star/basic.lean:127:0:
invalid simplification lemma 'star_mul'

Should I rename the field to something else, like antihomomorphism?

@eric-wieser
Copy link
Copy Markdown
Member

What's the plan WRT mathlib 4 here? Once this is approved, are you happy to forward-port?

@mans0954
Copy link
Copy Markdown
Collaborator Author

What's the plan WRT mathlib 4 here? Once this is approved, are you happy to forward-port?

@eric-wieser In principal yes - but what would the procedure be for forward-porting a PR when some of the files touched are in mathlib4 and others aren't?

@YaelDillies
Copy link
Copy Markdown
Collaborator

You only forward-port the changes to files that are already ported.

@mans0954
Copy link
Copy Markdown
Collaborator Author

Can't make sense of what's happened here. Where has star_add_monoid gone? May be easiest just to start again.

@YaelDillies
Copy link
Copy Markdown
Collaborator

I've renamed it to has_star_add

@kim-em kim-em added the too-late This PR was ready too late for inclusion in mathlib3 label Jul 16, 2023
@j-loreaux
Copy link
Copy Markdown
Collaborator

@mans0954 even though this is marked as too-late, I would like to see this generalization in mathlib4. If you are willing to PR it there, I would be happy to review it.

@mans0954
Copy link
Copy Markdown
Collaborator Author

@mans0954 even though this is marked as too-late, I would like to see this generalization in mathlib4. If you are willing to PR it there, I would be happy to review it.

Yes - I came to the conclusion that trying to get it into mathlib and forward port it to mathlib4 was too much hard work, and it would be better to wait until I could just add it straight to mathlib4.

Are we allowed to modify existing code in mathlib4 independent of mathlib now?

@mans0954
Copy link
Copy Markdown
Collaborator Author

@mans0954 even though this is marked as too-late, I would like to see this generalization in mathlib4. If you are willing to PR it there, I would be happy to review it.

@j-loreaux closing in favour of leanprover-community/mathlib4#6562

@mans0954 mans0954 closed this Aug 14, 2023
@YaelDillies YaelDillies deleted the non-assoc-star-mul branch August 14, 2023 09:21
bors bot pushed a commit to leanprover-community/mathlib4 that referenced this pull request Aug 31, 2023
… algebras (#6562)

Typically a * operation on a mathematical structure `R` equipped with a multiplication is an involutive anti-automorphism i.e.
```
∀ r s : R, star (r * s) = star s * star r
```
Currently mathlib defines a class `StarSemigroup` to be a semigroup satisfying this property. However, the requirement for the multiplication to be associative is unnecessarily restrictive. There are important classes of star-algebra which are not associative (e.g. JB*-algebras).

This PR removes the requirement for a  `StarSemigroup` to be a semigroup, merely requiring it to have a multiplication.

I've changed the name from `StarSemigroup` to `StarMul` since it's no longer a semigroup.

[Zulip discussion](https://leanprover.zulipchat.com/#narrow/stream/116395-maths/topic/non-associative.20*-algebras)

Previously opened as a mathlib PR leanprover-community/mathlib3#17949



Co-authored-by: Christopher Hoskin <[email protected]>
Co-authored-by: Eric Wieser <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

awaiting-review The author would like community review of the PR modifies-synchronized-file This PR touches a files that has already been ported to mathlib4, and may need a synchronization PR. 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.

5 participants