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: Define intNorm and intTrace. #9265

Closed
wants to merge 4 commits into from

Conversation

erdOne
Copy link
Member

@erdOne erdOne commented Dec 25, 2023


Open in Gitpod

@erdOne
Copy link
Member Author

erdOne commented Dec 25, 2023

The connection to RingOfIntegers.norm will be provided in a later PR.

@erdOne erdOne added awaiting-review The author would like community review of the PR t-algebra Algebra (groups, rings, fields etc) labels Dec 25, 2023
Mathlib/RingTheory/IntegralRestrict.lean Outdated Show resolved Hide resolved

section trace

/-- The restriction of the trace on `L/K` restricted onto `B/A` in an AKLB setup.
Copy link
Member

Choose a reason for hiding this comment

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

Is "AKLB setup" standard terminology (in mathlib)?
It's the first time I see it.

Copy link
Member Author

Choose a reason for hiding this comment

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

In this file, we assume `A` is an integrally closed domain; `K` is the fraction ring of `A`;
`L` is a finite (separable) extension of `K`; `B` is the integral closure of `A` in `L`.
We call this the AKLB setup.

is in the module docstring.

Copy link
Member

Choose a reason for hiding this comment

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

I see. That makes sense. But I'm not sure that we should assume this kind of info in docstrings, since they show up outside the context of the file.
Maybe add

Suggested change
/-- The restriction of the trace on `L/K` restricted onto `B/A` in an AKLB setup.
/-- The restriction of the trace on `L/K` restricted onto `B/A` in an AKLB setup (see module docstring).

for now...

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 think this terminology also appears in some literature, for example Ash's A Course In Algebraic Number Theory.
Maybe I can expand the docstring into a library note in a future PR and reference it here.
Have we implemented library note in lean4 yet?

Copy link
Member Author

@erdOne erdOne Jan 8, 2024

Choose a reason for hiding this comment

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

I kept the docstring as is for now. This is an auxiliary definition that probably won't be used outside this file.

@jcommelin jcommelin added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Jan 6, 2024
@riccardobrasca riccardobrasca assigned xroblot and unassigned xroblot Jan 8, 2024
@xroblot
Copy link
Collaborator

xroblot commented Jan 8, 2024

LGTM. How do the results in this PR relate to what Riccardo did in NumberTheory.NumberField.Norm?

Mathlib/RingTheory/IntegralRestrict.lean Outdated Show resolved Hide resolved
Mathlib/RingTheory/LocalProperties.lean Outdated Show resolved Hide resolved
Co-authored-by: Johan Commelin <johan@commelin.net>
Co-authored-by: Xavier Roblot <46200072+xroblot@users.noreply.github.com>
@erdOne
Copy link
Member Author

erdOne commented Jan 8, 2024

The connection needs some other stuff (for example #9444) and will come in a later PR.

@erdOne erdOne 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 Jan 8, 2024
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 ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Jan 9, 2024
mathlib-bors bot pushed a commit that referenced this pull request Jan 9, 2024
Co-authored-by: Andrew Yang <36414270+erdOne@users.noreply.github.com>
@mathlib-bors
Copy link

mathlib-bors bot commented Jan 9, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title feat: Define intNorm and intTrace. [Merged by Bors] - feat: Define intNorm and intTrace. Jan 9, 2024
@mathlib-bors mathlib-bors bot closed this Jan 9, 2024
@mathlib-bors mathlib-bors bot deleted the erd1/intnorm branch January 9, 2024 10:40
@riccardobrasca
Copy link
Member

I somehow missed this PR, sorry. Anyway the new intNorm could completely replace RingOfIntegers.norm, right?

@erdOne
Copy link
Member Author

erdOne commented Jan 9, 2024

Maybe yes. This might not work when k and K aren't number fields but we might not care.
I'd still keep RingOfIntegers.norm but redefine it in terms of intNorm though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

None yet

5 participants