Skip to content

Zlattices - primitive embedding with roots in orthogonal#5888

Merged
simonbrandhorst merged 11 commits intooscar-system:masterfrom
bpiroddi:dev_lattice
Apr 24, 2026
Merged

Zlattices - primitive embedding with roots in orthogonal#5888
simonbrandhorst merged 11 commits intooscar-system:masterfrom
bpiroddi:dev_lattice

Conversation

@bpiroddi
Copy link
Copy Markdown
Contributor

primitive embedding of a hyperbolic lattice in a unimodular of given rank st the orthogonal complement has the most roots

…nk st the orthogonal complement has the most roots
@bpiroddi bpiroddi marked this pull request as draft March 24, 2026 11:27
Comment thread test/NumberTheory/embedding_with_roots.jl Outdated
Comment thread src/NumberTheory/embedding_with_roots.jl Outdated
@simonbrandhorst
Copy link
Copy Markdown
Collaborator

I will review this, once it is ready :-). Thanks for the PR.

@simonbrandhorst simonbrandhorst marked this pull request as ready for review April 16, 2026 06:54
Copy link
Copy Markdown
Collaborator

@simonbrandhorst simonbrandhorst left a comment

Choose a reason for hiding this comment

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

Many thanks for this PR. It is a very good first draft.
I have just some minor stylistic remarks.

Comment thread src/NumberTheory/embedding_with_roots.jl Outdated
Comment thread src/NumberTheory/embedding_with_roots.jl Outdated
Comment thread src/NumberTheory/embedding_with_roots.jl
Comment thread src/NumberTheory/embedding_with_roots.jl Outdated
Comment thread src/NumberTheory/embedding_with_roots.jl Outdated
Comment thread src/NumberTheory/embedding_with_roots.jl Outdated
Comment thread src/NumberTheory/embedding_with_roots.jl Outdated
SR, inj = direct_sum(S, R)
iS, iR = inj
V = ambient_space(SR)
S = lattice(V, basis_matrix(S) * iS.matrix)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does iS(S) work?. If not, then I think we should make it work. (but not in this PR)

Comment thread src/NumberTheory/embedding_with_roots.jl Outdated
Comment thread src/NumberTheory/embedding_with_roots.jl
@simonbrandhorst simonbrandhorst added the release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes label Apr 22, 2026
Copy link
Copy Markdown
Collaborator

@simonbrandhorst simonbrandhorst left a comment

Choose a reason for hiding this comment

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

Just a final comment on the docstring :-).

Comment thread src/NumberTheory/embedding_with_roots.jl Outdated
Comment thread src/NumberTheory/embedding_with_roots.jl Outdated
bpiroddi and others added 2 commits April 23, 2026 15:01
Co-authored-by: Simon Brandhorst <brandhorst@math.uni-sb.de>
Co-authored-by: Simon Brandhorst <brandhorst@math.uni-sb.de>
@simonbrandhorst simonbrandhorst enabled auto-merge (squash) April 23, 2026 14:24
@simonbrandhorst simonbrandhorst merged commit 275fc15 into oscar-system:master Apr 24, 2026
39 of 41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants