QuadFormAndIsom: more features
#3160
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #3160 +/- ##
==========================================
+ Coverage 80.86% 81.00% +0.13%
==========================================
Files 550 553 +3
Lines 72847 73420 +573
==========================================
+ Hits 58909 59471 +562
- Misses 13938 13949 +11
|
|
@simonbrandhorst do you want to have an artificial look? |
|
|
||
| Given an integer lattice with isometry $(L, f)$, one often would like to know | ||
| the *spinor norm* of $f$ seen as an isometry of the real quadratic space $L\times | ||
| \mathbb{R}$. Since convention might change depending on the author, we allow to |
There was a problem hiding this comment.
Usually one would like to now the spinor norm over QQ and not just over R.
| Given a quadratic space with isometry $(V, f)$, one often would like to compute | ||
| the *spinor norm* of $f$ seen as an isometry of the real quadratic space | ||
| $V\otimes \mathbb{R}$. Depending on their convention, the users can choose to | ||
| compute such a norm with respect to the opposite form on $V$: |
There was a problem hiding this comment.
Better give a definition.
| compute such a norm with respect to the opposite form on $V$: | ||
|
|
||
| ```@docs | ||
| spinor_norm(::QuadSpaceWithIsom) |
There was a problem hiding this comment.
I disagree with the name. It should be "real_spinor_norm" or "spinor_norm" with an extra argument
There was a problem hiding this comment.
seem to me like this actually computes the rational spinor norm
There was a problem hiding this comment.
Yes indeed, since I was focusing on the sign, I wrote the wrong thing here, it is actually the rational spinor norm.
| # - `even` forces the primitive extensions to be even - if all the data w.r.t to | ||
| # M, N and q does not allow for a primitive extension to be even, an error is | ||
| # thrown; |
There was a problem hiding this comment.
meaning? Why not the empty list?
| # M, N and q does not allow for a primitive extension to be even, an error is | ||
| # thrown; | ||
| # - `exist_only` is meant only to state about the existence of a primitive | ||
| # extensions without doing further computations once it is proved to exist; |
There was a problem hiding this comment.
| # extensions without doing further computations once it is proved to exist; | |
| # extension without doing further computations once it is proved to exist; |
| # - `ext_type` is the type of extension: :p means "plain" (without isometry) and | ||
| # :e means "equivariant". The pair of symbol depends whether we consider M and |
There was a problem hiding this comment.
try to avoid abbreviations for better readability of the code, call the symbols :plain and :equivariant
| # | ||
| # This generic function is then call by the different methods for primitive | ||
| # extensions later | ||
| function _primitive_extensions_generic( |
There was a problem hiding this comment.
this function has a very long body. Would it make sense to split is up into sever functions?
| The content of $V$ depends on the value `classification`. There are 6 possibilities: | ||
| - `classification == "none"`: $V$ is the empty list; | ||
| - `classification == "first"`: $V$ consists of the first primitive extension computed; | ||
| - `classification == "subsub"`: $V$ consists of representatives for all isomorphism classes of primitive extensions of $M\oplus N$, up to the actions of $O(M)$ and $O(N)$; |
There was a problem hiding this comment.
| - `classification == "subsub"`: $V$ consists of representatives for all isomorphism classes of primitive extensions of $M\oplus N$, up to the actions of $O(M)$ and $O(N)$; | |
| - `classification == "subsub"`: $V$ consists of representatives for all isomorphism classes of primitive extensions of $M\oplus N$ satisfying the given conditions, up to the actions of $O(M)$ and $O(N)$; |
and likewise below
|
|
||
| If `check` is set to `true`, the function determines whether $L$ is in fact unique | ||
| in its genus. | ||
|
|
||
| We follow the algorithm described in the proof of Proposition 1.15.1 of | ||
| [Nik79](@cite). The classification methods for the symbols `:sublat` and `:emb` | ||
| [Nik79](@cite). The classification methods for the symbols `"sub"` and `"emb"` |
There was a problem hiding this comment.
Do we have a guideline whether to use strings or symbols for keyword arguments?
There was a problem hiding this comment.
Most other parts of the code I know of use symbols
| @req dim(Vf) > 0 "V must have positive dimension" | ||
| D, U = Hecke._gram_schmidt(gram_matrix(Vf), QQ) | ||
| fD = U*isometry(Vf)*inv(U) | ||
| return Oscar.spin(s*D, fD) |
There was a problem hiding this comment.
| return Oscar.spin(s*D, fD) | |
| return spin(s*D, fD) |
what is spin computing?
There was a problem hiding this comment.
The (rational) spinor norm of fD with respect to the diagonal Gram matrix s*D. The implementation only works for orthogonal basis, this is why I have to do a Gram-Schmidt orthogonalisation first.
|
We discussed with @simonbrandhorst about another feature I could bring for infinite isometries (let us say extend some current features to a more general context). Since this PR is already quite heavy, I will keep that for another time. |
spinor_norm;