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

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

Closed
wants to merge 36 commits into from

Conversation

mans0954
Copy link
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

@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
src/algebra/star/pi.lean Outdated Show resolved Hide resolved
src/algebra/star/pi.lean Outdated Show resolved Hide resolved
mans0954 and others added 2 commits December 15, 2022 13:42
Co-authored-by: Yaël Dillies <yael.dillies@gmail.com>
@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
@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
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
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
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
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
Member

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

@mans0954
Copy link
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
Collaborator

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

@mans0954
Copy link
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
Collaborator

I've renamed it to has_star_add

@semorrison semorrison added the too-late This PR was ready too late for inclusion in mathlib3 label Jul 16, 2023
@j-loreaux
Copy link
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
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
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/mathlib#17949



Co-authored-by: Christopher Hoskin <mans0954@users.noreply.github.com>
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
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.

None yet

5 participants