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] - feat(number_theory/legendre_symbol/mul_character): alternative implementation #14768

Closed
wants to merge 13 commits into from

Conversation

MichaelStollBayreuth
Copy link
Collaborator

@MichaelStollBayreuth MichaelStollBayreuth commented Jun 16, 2022

This is an alternative version of number_theory/legendre_symbol/mul_character.lean.

It defines mul_character R R' as a monoid_hom that sends non-units to zero.
This allows to define a comm_group structure on mul_character R R'.

There is an alternative implementation in #14716 (side by side comparison).

See the discussion on Zulip.


Open in Gitpod

@MichaelStollBayreuth MichaelStollBayreuth added WIP Work in progress awaiting-review The author would like community review of the PR labels Jun 16, 2022
@MichaelStollBayreuth MichaelStollBayreuth removed the WIP Work in progress label Jun 23, 2022
Copy link
Member

@kbuzzard kbuzzard left a comment

Choose a reason for hiding this comment

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

This looks great to me -- thanks very much Michael! Dirichlet characters, a special case of this, will be a very welcome addition to mathlib.

src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
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.

Unfortunately I couldn't finish the review today, the shops are closing soon.

These may look like a lot of suggestions, but most are relatively minor and repeated a few times.

src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
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.

Thank you for your work! It looks good to me with these changes.

bors d+

src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
src/number_theory/legendre_symbol/mul_character.lean Outdated Show resolved Hide resolved
@bors
Copy link

bors bot commented Jun 29, 2022

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

@leanprover-community-bot-assistant leanprover-community-bot-assistant added delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Jun 29, 2022
@MichaelStollBayreuth
Copy link
Collaborator Author

bors r+

bors bot pushed a commit that referenced this pull request Jun 30, 2022
…entation (#14768)

This is an alternative version of `number_theory/legendre_symbol/mul_character.lean`.

It defines `mul_character R R'` as a `monoid_hom` that sends non-units to zero.
This allows to define a `comm_group` structure on `mul_character R R'`.

There is an alternative implementation in #14716 ([side by side comparison](legendre_symbol_mul_char...variant)).

See the [discussion on Zulip](https://leanprover.zulipchat.com/#narrow/stream/116395-maths/topic/Implementation.20of.20multiplicative.20characters).
@bors
Copy link

bors bot commented Jun 30, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(number_theory/legendre_symbol/mul_character): alternative implementation [Merged by Bors] - feat(number_theory/legendre_symbol/mul_character): alternative implementation Jun 30, 2022
@bors bors bot closed this Jun 30, 2022
@bors bors bot deleted the variant branch June 30, 2022 07:34
bors bot pushed a commit that referenced this pull request Jul 18, 2022
…as `mul_char`s (#15418)

This is a follow-up to #14768; it defines quadratic characters (and also the characters on `zmod 4` and `zmod 8`) to be of type `mul_char F int` (where `F` is a finite field).

Some content of `number_theory/legendre_symbol/quadratic_char` is moved within the file; one proof is replaced by directly using the corresponding more general result.

See here on [Zulip](https://leanprover.zulipchat.com/#narrow/stream/116395-maths/topic/Implementation.20of.20multiplicative.20characters/near/289635166).
bors bot pushed a commit that referenced this pull request Jul 18, 2022
…as `mul_char`s (#15418)

This is a follow-up to #14768; it defines quadratic characters (and also the characters on `zmod 4` and `zmod 8`) to be of type `mul_char F int` (where `F` is a finite field).

Some content of `number_theory/legendre_symbol/quadratic_char` is moved within the file; one proof is replaced by directly using the corresponding more general result.

See here on [Zulip](https://leanprover.zulipchat.com/#narrow/stream/116395-maths/topic/Implementation.20of.20multiplicative.20characters/near/289635166).
bors bot pushed a commit that referenced this pull request Jul 18, 2022
…as `mul_char`s (#15418)

This is a follow-up to #14768; it defines quadratic characters (and also the characters on `zmod 4` and `zmod 8`) to be of type `mul_char F int` (where `F` is a finite field).

Some content of `number_theory/legendre_symbol/quadratic_char` is moved within the file; one proof is replaced by directly using the corresponding more general result.

See here on [Zulip](https://leanprover.zulipchat.com/#narrow/stream/116395-maths/topic/Implementation.20of.20multiplicative.20characters/near/289635166).
joelriou pushed a commit that referenced this pull request Jul 23, 2022
…as `mul_char`s (#15418)

This is a follow-up to #14768; it defines quadratic characters (and also the characters on `zmod 4` and `zmod 8`) to be of type `mul_char F int` (where `F` is a finite field).

Some content of `number_theory/legendre_symbol/quadratic_char` is moved within the file; one proof is replaced by directly using the corresponding more general result.

See here on [Zulip](https://leanprover.zulipchat.com/#narrow/stream/116395-maths/topic/Implementation.20of.20multiplicative.20characters/near/289635166).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated The PR author may merge after reviewing final suggestions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants