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(Algebra/Star/*): Allow for star operation on non-associative algebras #6562

Closed
wants to merge 21 commits into from

Conversation

mans0954
Copy link
Collaborator

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

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


Open in Gitpod

@eric-wieser
Copy link
Member

!bench

Mathlib/Algebra/Star/Basic.lean Outdated Show resolved Hide resolved
Mathlib/Algebra/Star/Basic.lean Outdated Show resolved Hide resolved
mans0954 and others added 2 commits August 14, 2023 17:37
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@eric-wieser
Copy link
Member

I was hoping the benchmark bot would respond; maybe it crashed?

!bench

@eric-wieser
Copy link
Member

!bench

@j-loreaux j-loreaux added the t-algebra Algebra (groups, rings, fields etc) label Aug 16, 2023
@j-loreaux j-loreaux self-requested a review August 16, 2023 16:17
@eric-wieser
Copy link
Member

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 6afc5de.Found no runs to compare against.

@eric-wieser
Copy link
Member

eric-wieser commented Aug 21, 2023

I've merged in a commit from master that I know has a benchmark, in the hope that it produces a comparison.

@eric-wieser
Copy link
Member

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 1362a3a.
There were significant changes against commit f25139b:

  Benchmark                                           Metric         Change
  =========================================================================
+ ~Mathlib.Analysis.InnerProductSpace.Adjoint         instructions    -5.3%
+ ~Mathlib.Analysis.NormedSpace.Star.GelfandDuality   instructions    -8.0%
- ~Mathlib.Analysis.VonNeumannAlgebra.Basic           instructions    17.3%

@mans0954
Copy link
Collaborator Author

@eric-wieser do we need to do something as a result of the benchmarks?

@eric-wieser
Copy link
Member

No, I think it's probably fine; I just wanted to check it didn't majorly slow down every file about star operations.

I'll let @j-loreaux give the final review on this,

Copy link
Collaborator

@j-loreaux j-loreaux left a comment

Choose a reason for hiding this comment

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

Thanks! Sorry for the slow review. It would be good to see where existing variable declarations involving StarRing could have the hypotheses weakened to the NonAssoc variants, but that's for another PR.

Mathlib/Algebra/Star/NonUnitalSubalgebra.lean Outdated Show resolved Hide resolved
@j-loreaux
Copy link
Collaborator

bors d+

@bors
Copy link

bors bot commented Aug 30, 2023

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

@github-actions github-actions bot added delegated and removed awaiting-review The author would like community review of the PR labels Aug 30, 2023
@mans0954
Copy link
Collaborator Author

bors r+

bors bot pushed a commit 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>
@bors
Copy link

bors bot commented Aug 31, 2023

Pull request successfully merged into master.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title refactor(Algebra/Star/*): Allow for star operation on non-associative algebras [Merged by Bors] - refactor(Algebra/Star/*): Allow for star operation on non-associative algebras Aug 31, 2023
@bors bors bot closed this Aug 31, 2023
@bors bors bot deleted the mans0954/non-assoc-star-mul branch August 31, 2023 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated t-algebra Algebra (groups, rings, fields etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants