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

feat(ring_theory/algebraic): algebraic extensions, algebraic elements #1519

Merged
merged 35 commits into from
Oct 16, 2019

Conversation

jcommelin
Copy link
Member

@jcommelin jcommelin commented Oct 8, 2019

TO CONTRIBUTORS:

Make sure you have:

  • reviewed and applied the coding style: coding, naming
  • reviewed and applied the documentation requirements
  • make sure definitions and lemmas are put in the right files
  • make sure definitions and lemmas are not redundant

If this PR is related to a discussion on Zulip, please include a link in the discussion.

For reviewers: code review check list

src/data/polynomial.lean Outdated Show resolved Hide resolved
@@ -166,7 +167,7 @@ begin
rw [finsupp.sum], refine sum_mem _ _, intros r2 hr2,
rw [finsupp.sum], refine sum_mem _ _, intros r1 hr1,
rw [algebra.mul_smul_comm, algebra.smul_mul_assoc],
letI : module ↥S₀ A := _inst_6, refine smul_mem _ _ (smul_mem _ _ _),
letI hS₀A : module ↥S₀ A := ‹_›, refine smul_mem _ _ (smul_mem _ _ _),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Getting rid of _inst_6 is certainly an improvement but this could still be clearer. For me it seems to work even without any letI on this line at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've made some aesthetic changes to this proof. If I delete this particular letI, the proof breaks.

src/ring_theory/integral_closure.lean Outdated Show resolved Hide resolved
src/ring_theory/algebraic.lean Outdated Show resolved Hide resolved
src/ring_theory/algebraic.lean Outdated Show resolved Hide resolved
src/data/polynomial.lean Show resolved Hide resolved
src/ring_theory/integral_closure.lean Show resolved Hide resolved
(S : set (comap R A B))
(hS : S = (↑((finset.range (p.nat_degree + 1)).image
(λ i, to_comap R A B (p.coeff i))) : set (comap R A B))) :
is_integral (adjoin R S) (comap.to_comap R A B x) :=
Copy link
Collaborator

Choose a reason for hiding this comment

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

By the way, this lemma is basically trivial and doesn't use A_alg right?

Copy link
Member Author

@jcommelin jcommelin Oct 9, 2019

Choose a reason for hiding this comment

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

Yep, I've inlined the include where it's actually needed. And indeed, this lemma is trivial. That's why it has the longest proof (-;

src/ring_theory/integral_closure.lean Outdated Show resolved Hide resolved
src/ring_theory/integral_closure.lean Outdated Show resolved Hide resolved
@jcommelin jcommelin added the awaiting-review The author would like community review of the PR label Oct 9, 2019
variables {R}

/-- A subalgebra is algebraic if all its elements are algebraic. -/
def subalgebra.is_algebraic (S : subalgebra R A) : Prop := ∀ x ∈ S, is_algebraic R x
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason to introduce is_algebraic for subalgebras first? It looks like "compact subset" again.
Why not just define algebra.is_algebraic directly and then say a subalgebra is algebraic if it is algebraic as an algebra?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Either way I think you will eventually want to prove that a subalgebra S is algebraic in the sense of algebra.is_algebraic if and only if it is algebraic in the sense of this current subalgebra.is_algebraic (right?). To me algebra.is_algebraic seems like the primary notion so defining it directly looks better.

Copy link
Member Author

Choose a reason for hiding this comment

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

More or less done. They now all have elementwise definitions and iffs relating the various definitions.

@sgouezel
Copy link
Collaborator

You already had a bunch of comments, so I think it is good to go.

@sgouezel sgouezel added the ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) label Oct 16, 2019
@mergify mergify bot merged commit ee863ec into master Oct 16, 2019
@mergify mergify bot deleted the algebraic-trans branch October 16, 2019 21:41
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 15, 2020
…leanprover-community#1519)

* chore(ring_theory/algebra): make first type argument explicit in alg_hom

Now this works, and it didn't work previously even with `@`
```lean
structure alg_equiv (α β γ : Type*) [comm_ring α] [ring β] [ring γ]
  [algebra α β] [algebra α γ] extends alg_hom α β γ :=
```

* Update algebra.lean

* feat(field_theory/algebraic_closure)

* Remove sorries about minimal polynomials

* Define alg_equiv.symm

* typo

* Remove another sorry, in base_extension

* Work in progress

* Remove a sorry in maximal_extension_chain

* Merge two sorries

* More sorries removed

* More work on transitivity of algebraicity

* WIP

* Sorry-free definition of algebraic closure

* More or less sorries

* Removing some sorries

* WIP

* Fix algebraic.lean

* Fix build, mostly

* Remove stuff about UMP of alg clos

* Prove transitivity of algebraic extensions

* Add some docstrings

* Remove files with stuff for future PRs

* Add a bit to the module docstring

* Fix module docstring

* Include assumption in section injective

* Aesthetic changes to is_integral_of_mem_of_fg

* Little improvements of proofs in algebraic.lean

* Improve some proofs in integral_closure.lean

* Make variable name explicit

* Process comments
@YaelDillies YaelDillies removed the awaiting-review The author would like community review of the PR label Oct 31, 2021
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+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants