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(RingTheory/LocalRing/Module): Finitely presented flat modules over local rings are free. #14375

Closed
wants to merge 9 commits into from

Conversation

erdOne
Copy link
Member

@erdOne erdOne commented Jul 3, 2024


Open in Gitpod

Copy link

github-actions bot commented Jul 3, 2024

PR summary 812755553c

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference
Mathlib.RingTheory.LocalRing.Module 1467

Declarations diff

+ LocalRing.split_injective_iff_lTensor_residueField_injective
+ TensorProduct.mk_surjective
+ free_of_flat_of_localRing
+ free_of_lTensor_residueField_injective
+ free_of_maximalIdeal_rTensor_injective
+ lTensor_injective_of_exact_of_exact_of_rTensor_injective
+ map_mkQ_eq
+ map_mkQ_eq_top
+ map_tensorProduct_mk_eq_top
+ span_eq_top_of_tmul_eq_basis
+ subsingleton_tensorProduct

You can run this locally as follows
## summary with just the declaration names:
./scripts/no_lost_declarations.sh short <optional_commit>

## more verbose report:
./scripts/no_lost_declarations.sh <optional_commit>

@erdOne erdOne added awaiting-review awaiting-CI t-algebra Algebra (groups, rings, fields, etc) labels Jul 3, 2024
Copy link
Collaborator

@faenuccio faenuccio left a comment

Choose a reason for hiding this comment

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

I have read the first 200 lines of Module.lean up to the declaration Module.free_of_lTensor_residueField_injective

@@ -419,6 +419,16 @@ theorem TensorProduct.map_ker :

variable (M)

variable (R) in
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any specific reason for putting R as a localized variable and leaving S inside the statement? What about having variable (R S) in before the theorem?

Copy link
Member Author

Choose a reason for hiding this comment

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

variable (R) in makes the variable explicit, but there is not a global variable {S} living around.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree, but variable (R S) has the same effect... Not very important, at any rate.

@faenuccio faenuccio self-assigned this Jul 3, 2024
Co-authored-by: Filippo A. E. Nuccio <[email protected]>
@erdOne erdOne added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review labels Jul 3, 2024
@erdOne erdOne added awaiting-review awaiting-CI and removed awaiting-author A reviewer has asked the author a question or requested changes labels Jul 4, 2024
Copy link
Collaborator

@faenuccio faenuccio left a comment

Choose a reason for hiding this comment

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

Thanks so much for all the work!

rw [← LinearMap.lTensor_comp, hl, LinearMap.lTensor_id]
exact Function.HasLeftInverse.injective ⟨_, LinearMap.congr_fun this⟩
· intro h
-- By the lemma above, `k ⊗ l` injective => `N ⧸ l(M)` free.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you rather refer to the name of the declaration instead of "lemma above"?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is directly on the next line though.
I've changed the wording a bit but I'll include the lemma name if you still think it's better to do so.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think so, because in the future people might move things around and it won't be "above" anymore (and given the lack of attention that is normally taken at the doc, it is likely they would forget to update).

Copy link
Member Author

Choose a reason for hiding this comment

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

To clarify, the current wording is

    -- By the a previously proved lemma, `k ⊗ l` injective => `N ⧸ l(M)` free.
    have := Module.free_of_lTensor_residueField_injective l (LinearMap.range l).mkQ

And you suggest

    -- By `Module.free_of_lTensor_residueField_injective`, `k ⊗ l` injective => `N ⧸ l(M)` free.
    have := Module.free_of_lTensor_residueField_injective l (LinearMap.range l).mkQ

Copy link
Collaborator

@faenuccio faenuccio Jul 5, 2024

Choose a reason for hiding this comment

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

Yes, thanks!

@faenuccio faenuccio added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review labels Jul 4, 2024
@erdOne erdOne added awaiting-review and removed awaiting-author A reviewer has asked the author a question or requested changes labels Jul 4, 2024
@faenuccio
Copy link
Collaborator

maintainer merge

Copy link

github-actions bot commented Jul 5, 2024

🚀 Pull request has been placed on the maintainer queue by faenuccio.

@erdOne
Copy link
Member Author

erdOne commented Jul 5, 2024

Thanks for the swift review!

Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

Thanks 🎉

bors merge

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the ready-to-merge This PR has been sent to bors. label Jul 5, 2024
rw [(LinearMap.exact_subtype_ker_map i).linearMap_comp_eq_zero]
simp only [LinearMap.lTensor_zero, LinearMap.zero_apply, map_zero]

theorem free_of_flat_of_localRing [Module.FinitePresentation R P] [Module.Flat R P] :
Copy link
Member

Choose a reason for hiding this comment

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

I think this is true for any finite module, maybe we can add a TODO (the proof is much harder IIRC).

Copy link
Member Author

Choose a reason for hiding this comment

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

We're actually not that far away. We already have the equational criterion of flatness and finite flat = free is a (relatively) easy application of that. Anyways I added a TODO.

@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Jul 5, 2024

Canceled.

@riccardobrasca
Copy link
Member

bors d+

@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Jul 5, 2024

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

@erdOne
Copy link
Member Author

erdOne commented Jul 5, 2024

bors merge

mathlib-bors bot pushed a commit that referenced this pull request Jul 5, 2024
@mathlib-bors
Copy link
Contributor

mathlib-bors bot commented Jul 5, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat(RingTheory/LocalRing/Module): Finitely presented flat modules over local rings are free. [Merged by Bors] - feat(RingTheory/LocalRing/Module): Finitely presented flat modules over local rings are free. Jul 5, 2024
@mathlib-bors mathlib-bors bot closed this Jul 5, 2024
@mathlib-bors mathlib-bors bot deleted the erd1/localflat branch July 5, 2024 09:32
@adomani adomani mentioned this pull request Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated maintainer-merge ready-to-merge This PR has been sent to bors. t-algebra Algebra (groups, rings, fields, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants