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(field_theory/finite): Chevalley–Warning #1564

Closed
wants to merge 81 commits into from

Conversation

jcommelin
Copy link
Member

@jcommelin jcommelin commented Oct 17, 2019


@rwbarton
Copy link
Collaborator

I would be inclined to split off the items numbered 4 and 5 from field_theory.finite into a new file. (Maybe 3 as well, but probably not.) That is, make field_theory.finite.{basic,chevalley_warning,default}.

@bryangingechen
Copy link
Collaborator

The merge conflict is from this code, added to the deleted field_theory.finite in #1554: https://github.com/leanprover-community/mathlib/blob/master/src/field_theory/finite.lean#L49-L87

@rwbarton
Copy link
Collaborator

Thanks Bryan, I'll try to glue the pieces back together.

Copy link
Collaborator

@rwbarton rwbarton left a comment

Choose a reason for hiding this comment

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

Oof, I underestimated how sticky this situation got. Sorry, I'm partly responsible for this mess. At this point, I think it is best to proceed in small steps. I suggest not doing any refactoring or variable renaming in this PR, and just adding the new content to a new file. I think the new content in field_theory.finite.basic is sum_pow_units, cast_card_eq_zero, sum_pow_lt_card_sub_one; probably you can verify this easily.

Since I'm already in the process of looking at this, I'll review it as though this has already happened.

src/algebra/big_operators.lean Outdated Show resolved Hide resolved
src/data/fintype.lean Outdated Show resolved Hide resolved
src/field_theory/finite/basic.lean Outdated Show resolved Hide resolved
have hq : 0 < q - 1,
{ rw [← card_units, fintype.card_pos_iff],
exact ⟨1⟩ },
cases is_cyclic.exists_generator (units K) with a ha,
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could write

let ⟨a, ha⟩ := is_cyclic.exists_generator (units K) in
calc ...

and avoid "fake tactic mode".

Copy link
Member Author

@jcommelin jcommelin Apr 26, 2020

Choose a reason for hiding this comment

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

Mwah... there is so much tactic mode in this proof that I don't care about a little bit of fake tactic mode as well. (I know that calc is a fake tactic. But all the begin ... end blocks aren't)
Anyway, I can fix it if you prefer.

src/field_theory/finite/basic.lean Outdated Show resolved Hide resolved
src/field_theory/finite/chevalley_warning.lean Outdated Show resolved Hide resolved
src/field_theory/finite/chevalley_warning.lean Outdated Show resolved Hide resolved
src/field_theory/finite/chevalley_warning.lean Outdated Show resolved Hide resolved
src/field_theory/finite/chevalley_warning.lean Outdated Show resolved Hide resolved
src/field_theory/finite/chevalley_warning.lean Outdated Show resolved Hide resolved
@rwbarton rwbarton added the awaiting-author A reviewer has asked the author a question or requested changes label Oct 24, 2019
@jcommelin
Copy link
Member Author

@rwbarton Thanks a lot for the thorough review. I currently don't have any time to work on this. What I propose is that we first fix the nat.prime is a class issue. Then clean up field_theory.finite{*,/*}. Once the library is in a stable state, we return to this PR. I'll label this WIP for the time being.

@jcommelin
Copy link
Member Author

jcommelin commented Apr 24, 2020

#2511 is a first step towards untangling the mess identified above.
#2518 is the second step.

bors bot pushed a commit that referenced this pull request Apr 24, 2020
…c search (#2511)

This PR introduces the class `fact P := P` for `P : Prop`, which is a way to inject elementary propositions into the type class system. This desing is inspired by @cipher1024 's https://gist.github.com/cipher1024/9bd785a75384a4870b331714ec86ad6f#file-zmod_redesign-lean.

We use this to endow `zmod p` with a `field` instance under the assumption `[fact p.prime]`.
As a consequence, the type `zmodp` is no longer needed, which removes a lot of duplicate code.
Besides that, we redefine `zmod n`, so that `n` is a natural number instead of a *positive* natural number, and `zmod 0` is now definitionally the integers.
To preserve computational properties, `zmod n` is not defined as quotient type, but rather as `int` and `fin n`, depending on whether `n` is `0` or `(k+1)`.

The rest of this PR is adapting the library to the new definitions (most notably quadratic reciprocity and Lagrange's four squares theorem).

Future work: Refactor the padics to use `[fact p.prime]` instead of making `nat.prime` a class in those files. This will address #1601 and #1648. Once that is done, we can clean up the mess in `char_p` (where the imports are really tangled) and finally get some movement in #1564.
@rwbarton
Copy link
Collaborator

Also, I would still prefer to remove those instances.

@jcommelin
Copy link
Member Author

@rwbarton I think I've done everything you suggested.

@jcommelin jcommelin added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels May 27, 2020
@PatrickMassot
Copy link
Member

Is this really a PR asking to merge 79 commits? Or is there some git accident here?

@bryangingechen
Copy link
Collaborator

Is this really a PR asking to merge 79 commits? Or is there some git accident here?

Many PRs were split off from this one. Note that in the current diff, only 4 files are changed.

@semorrison
Copy link
Collaborator

bors merge

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Jun 15, 2020
@bryangingechen
Copy link
Collaborator

bors r-
I'll fix the build and re-add it to the queue.

@bors
Copy link

bors bot commented Jun 15, 2020

Canceled.

@bryangingechen
Copy link
Collaborator

bors r+

bors bot pushed a commit that referenced this pull request Jun 15, 2020
@bors
Copy link

bors bot commented Jun 15, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(field_theory/finite): Chevalley–Warning [Merged by Bors] - feat(field_theory/finite): Chevalley–Warning Jun 15, 2020
@bors bors bot closed this Jun 15, 2020
Classification of quadratic forms over Q automation moved this from In progress to Done Jun 15, 2020
@bors bors bot deleted the chev-warn branch June 15, 2020 03:24
cipher1024 pushed a commit to cipher1024/mathlib that referenced this pull request Mar 15, 2022
…c search (leanprover-community#2511)

This PR introduces the class `fact P := P` for `P : Prop`, which is a way to inject elementary propositions into the type class system. This desing is inspired by @cipher1024 's https://gist.github.com/cipher1024/9bd785a75384a4870b331714ec86ad6f#file-zmod_redesign-lean.

We use this to endow `zmod p` with a `field` instance under the assumption `[fact p.prime]`.
As a consequence, the type `zmodp` is no longer needed, which removes a lot of duplicate code.
Besides that, we redefine `zmod n`, so that `n` is a natural number instead of a *positive* natural number, and `zmod 0` is now definitionally the integers.
To preserve computational properties, `zmod n` is not defined as quotient type, but rather as `int` and `fin n`, depending on whether `n` is `0` or `(k+1)`.

The rest of this PR is adapting the library to the new definitions (most notably quadratic reciprocity and Lagrange's four squares theorem).

Future work: Refactor the padics to use `[fact p.prime]` instead of making `nat.prime` a class in those files. This will address leanprover-community#1601 and leanprover-community#1648. Once that is done, we can clean up the mess in `char_p` (where the imports are really tangled) and finally get some movement in leanprover-community#1564.
cipher1024 pushed a commit to cipher1024/mathlib that referenced this pull request Mar 15, 2022
cipher1024 pushed a commit to cipher1024/mathlib that referenced this pull request Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Development

Successfully merging this pull request may close these issues.

None yet

8 participants