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] - chore: Slightly modify the split of CanonicalEmbedding #11917

Closed
wants to merge 4 commits into from

Conversation

xroblot
Copy link
Collaborator

@xroblot xroblot commented Apr 5, 2024

Create a new directory for the split of CanonicalEmbedding since more material is coming.

Also add some docstrings.


Open in Gitpod

@xroblot xroblot added awaiting-review The author would like community review of the PR easy < 20s of review time. See the lifecycle page for guidelines. awaiting-CI t-number-theory Number theory (also use t-algebra or t-analysis to specialize) labels Apr 5, 2024
Copy link
Collaborator

@Ruben-VandeVelde Ruben-VandeVelde left a comment

Choose a reason for hiding this comment

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

lgtm, one comment

@@ -507,5 +507,3 @@ theorem mem_span_fractionalIdealLatticeBasis (x : (E K)) :
rfl

end integerLattice

end NumberField.mixedEmbedding
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know it's not necessary, strictly speaking, but I'd keep this line.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. you're right.

Copy link
Member

@fpvandoorn fpvandoorn left a comment

Choose a reason for hiding this comment

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

LGTM

bors d+

The canonical embedding of a number field `K` of degree `n` is the ring homomorphism
`K →+* ℂ^n` that sends `x ∈ K` to `(φ_₁(x),...,φ_n(x))` where the `φ_i`'s are the complex
embeddings of `K`. Note that we do not choose an ordering of the embeddings, but instead map `K`
into the type `(K →+* ℂ) → ℂ` of `ℂ`-vectors indexed by the complex embeddings.
Copy link
Member

Choose a reason for hiding this comment

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

Was this intentionally duplicated from the Basic file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, you're right, that's not optimal. I'll change to a better docstring. Thanks

@mathlib-bors
Copy link

mathlib-bors bot commented Apr 5, 2024

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

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added delegated and removed awaiting-review The author would like community review of the PR labels Apr 5, 2024
@xroblot
Copy link
Collaborator Author

xroblot commented Apr 5, 2024

bors +

@mathlib-bors
Copy link

mathlib-bors bot commented Apr 5, 2024

Did you mean "r+"?

@xroblot
Copy link
Collaborator Author

xroblot commented Apr 5, 2024

bors r+

mathlib-bors bot pushed a commit that referenced this pull request Apr 5, 2024
Create a new directory for the split of `CanonicalEmbedding` since more material is coming.

Also add some docstrings.
@mathlib-bors
Copy link

mathlib-bors bot commented Apr 5, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: Slightly modify the split of CanonicalEmbedding [Merged by Bors] - chore: Slightly modify the split of CanonicalEmbedding Apr 5, 2024
@mathlib-bors mathlib-bors bot closed this Apr 5, 2024
@mathlib-bors mathlib-bors bot deleted the xfr-split_canonemb branch April 5, 2024 23:45
Louddy pushed a commit that referenced this pull request Apr 15, 2024
Create a new directory for the split of `CanonicalEmbedding` since more material is coming.

Also add some docstrings.
atarnoam pushed a commit that referenced this pull request Apr 16, 2024
Create a new directory for the split of `CanonicalEmbedding` since more material is coming.

Also add some docstrings.
uniwuni pushed a commit that referenced this pull request Apr 19, 2024
Create a new directory for the split of `CanonicalEmbedding` since more material is coming.

Also add some docstrings.
callesonne pushed a commit that referenced this pull request Apr 22, 2024
Create a new directory for the split of `CanonicalEmbedding` since more material is coming.

Also add some docstrings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated easy < 20s of review time. See the lifecycle page for guidelines. t-number-theory Number theory (also use t-algebra or t-analysis to specialize)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants