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: port FieldTheory.SplittingField.Construction #4891

Closed

Conversation

ChrisHughes24
Copy link
Member

@ChrisHughes24 ChrisHughes24 commented Jun 9, 2023


I've started a slightly different approach to this PR. So far I've constructed a SplittingFieldAux without worrying about whether the instances satisfy nice definitional equalities. Then the actual SplittingField is defined to be a quotient of an MvPolynomial ring by the kernel of the obvious map into SplittingFieldAux. Because the actual SplittingField will be a quotient of a MvPolynomial it should have nice instances on it.

This is just a suggestion, if somebody prefers doing it the old way and reviving the old PR, I won't object to that.

Open in Gitpod

Mathbin -> Mathlib
fix certain import statements
move "by" to end of line
add import to Mathlib.lean
@ChrisHughes24 ChrisHughes24 added mathlib-port This is a port of a theory file from mathlib. WIP Work in progress labels Jun 9, 2023
@Ruben-VandeVelde
Copy link
Collaborator

@ChrisHughes24 in case you forgot, there may be fixes in #4339 that can be salvaged

@ChrisHughes24
Copy link
Member Author

@ChrisHughes24 in case you forgot, there may be fixes in #4339 that can be salvaged

I didn't forget. I'm going to claim this for about a day I think. I have an idea about how to tidy it up a lot.

@ChrisHughes24 ChrisHughes24 added help-wanted The author needs attention to resolve issues and removed WIP Work in progress labels Jun 9, 2023
@jcommelin jcommelin added the awaiting-review The author would like community review of the PR label Jun 13, 2023
@semorrison
Copy link
Contributor

LGTM.

@ChrisHughes24
Copy link
Member Author

Maybe we should add a co authored by because I stole code from the previous PR.

@riccardobrasca
Copy link
Member

Should we wait to fix the mathlib3 problems before merging this? The relevant PR is #19178.

@eric-wieser
Copy link
Member

eric-wieser commented Jun 13, 2023

I think we should also forward-port #19179 first. The version of those changes in this PR is not identical to the ones in mathlib3, and a forward-port PR would create an easy place to align them

@semorrison semorrison added the blocked-by-other-PR This PR depends on another PR which is still in the queue. label Jun 13, 2023
@semorrison semorrison removed the blocked-by-other-PR This PR depends on another PR which is still in the queue. label Jun 14, 2023
@semorrison
Copy link
Contributor

This PR/issue depends on:

bors bot pushed a commit to leanprover-community/mathlib that referenced this pull request Jun 15, 2023
We refactor the definition of `splitting_field`. The main motivation is to backport [!4#4891](leanprover-community/mathlib4#4891) since the is seems very problematic to port the current design.


Zulip discussion relevant to this PR: https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/!4.234891.20.20FieldTheory.2ESplittingField
@jcommelin
Copy link
Member

Now that Lean 3 is happy with the corresponding refactor, let's get this merged!

@jcommelin
Copy link
Member

bors r+

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Jun 15, 2023
bors bot pushed a commit that referenced this pull request Jun 15, 2023
Co-authored-by: Scott Morrison <scott.morrison@anu.edu.au>
Co-authored-by: Johan Commelin <johan@commelin.net>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@bors
Copy link

bors bot commented Jun 15, 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 feat: port FieldTheory.SplittingField.Construction [Merged by Bors] - feat: port FieldTheory.SplittingField.Construction Jun 15, 2023
@bors bors bot closed this Jun 15, 2023
@bors bors bot deleted the port/FieldTheory.SplittingField.Construction branch June 15, 2023 08:04
Comment on lines +153 to +159
--Porting note: new instance
instance isScalarTower_right [CommSemiring R] [CommSemiring S₁] [DistribSMul R S₁]
[IsScalarTower R S₁ S₁] : IsScalarTower R (MvPolynomial σ S₁) (MvPolynomial σ S₁) :=
⟨by
rintro x p q
dsimp [MvPolynomial] at p q ⊢
rw [smul_mul_assoc]⟩
Copy link
Member

Choose a reason for hiding this comment

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

This is replaced in #5070

bors bot pushed a commit that referenced this pull request Jun 15, 2023
A sloppy version of this was added in #4891. This adds the better version that we wrote in mathlib3.
alexkeizer pushed a commit that referenced this pull request Jun 22, 2023
Co-authored-by: Scott Morrison <scott.morrison@anu.edu.au>
Co-authored-by: Johan Commelin <johan@commelin.net>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
alexkeizer pushed a commit that referenced this pull request Jun 22, 2023
A sloppy version of this was added in #4891. This adds the better version that we wrote in mathlib3.
semorrison pushed a commit that referenced this pull request Jun 25, 2023
Co-authored-by: Scott Morrison <scott.morrison@anu.edu.au>
Co-authored-by: Johan Commelin <johan@commelin.net>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
semorrison pushed a commit that referenced this pull request Jun 25, 2023
A sloppy version of this was added in #4891. This adds the better version that we wrote in mathlib3.
bors bot pushed a commit that referenced this pull request Sep 4, 2023
…mmute (#6734)

A similar trick to the trick used in #4891 makes all the required type class diagrams commute.

I also added instances for `CharP` and `CharZero` and tests for the instance diagrams.



Co-authored-by: Chris Hughes <33847686+ChrisHughes24@users.noreply.github.com>
ebab pushed a commit that referenced this pull request Sep 6, 2023
…mmute (#6734)

A similar trick to the trick used in #4891 makes all the required type class diagrams commute.

I also added instances for `CharP` and `CharZero` and tests for the instance diagrams.



Co-authored-by: Chris Hughes <33847686+ChrisHughes24@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mathlib-port This is a port of a theory file from mathlib. ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants