Opened 6 years ago
Last modified 6 years ago
#22494 needs_work enhancement
ramification properties of quaternion algebras
Reported by: | Aurel Page | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.6 |
Component: | number theory | Keywords: | days84, quaternion algebras, hilbert symbol, ramification |
Cc: | Alyson Deines, Gonzalo Tornaría, Marc Masdeu | Merged in: | |
Authors: | Aurel Page | Reviewers: | |
Report Upstream: | N/A | Work issues: | |
Branch: | u/aurel/quatalg_ramification (Commits, GitHub, GitLab) | Commit: | a828d9e5eb56934f185d9b49b29edb40105f1958 |
Dependencies: | Stopgaps: |
Description
Improvements:
- no duplicate factorisations (ramified_primes)
- doc correctly states that ramified_primes is implemented over number fields
- add ramified_infinite_places
- is_division_algebra over number fields
- is_matrix_ring over number fields
Change History (6)
comment:1 Changed 6 years ago by
Branch: | → u/aurel/quatalg_ramification |
---|
comment:2 Changed 6 years ago by
Cc: | Alyson Deines Gonzalo Tornaría Marc Masdeu added |
---|---|
Commit: | → a828d9e5eb56934f185d9b49b29edb40105f1958 |
Status: | new → needs_review |
comment:3 Changed 6 years ago by
I am not sure about the list vs set choice in ramified_primes and ramified_infinite_places. Tell me what I should do!
comment:4 Changed 6 years ago by
Keywords: | days84 added |
---|
comment:5 Changed 6 years ago by
Status: | needs_review → needs_work |
---|
Hello,
some quick comments regarding code clarity.
Why in the function ramified_primes(self)
do you need
raise ValueError("base field must be rational numbers or number field")
From what I understand it is not possible to construct a quaternion algebra over something else than a number field.
Are you sure one needs hilbert_ramification
in the global namespace (modif. in sage.arith.all
)? I would be in favor of removing all of hilbert_symbol
, hilbert_conductor
, hilbert_conductor_inverse
from the global namespace.
ram = set() for p in set().union([ZZ(2)], prime_divisors(a), prime_divisors(b)): if hilbert_symbol(a, b, p) == -1: ram.add(p) return ram
could be
return set(p for p in set().union([ZZ(2)], prime_divisors(a), prime_divisors(b)) if hilbert_symbol(a, b, p) == -1)
(up to you, not sure it is more readable)
Keep spaces around ==
. len(ram)==0
should be len(ram) == 0
.
Why do you use once set.union
and the other time union
?
comment:6 Changed 6 years ago by
Hello,
Thanks for your review.
It is possible to construct a quaternion algebra over something else than a number field: try
sage: K.<x> = FunctionField(QQ) sage: A = QuaternionAlgebra(K,-1,x) sage: A.ramified_primes()
But constructing it from its ramification only makes sense over certain fields, including number fields.
I am ok with removing hilbert_*
from the global namespace, but that breaks backwards compatibility: several of these function were defined before my patch. Should they be methods of QQ
or something else?
I will take into account your code suggestions.
The set.union
vs union
is because I simply moved around the existing code for the two versions of hilbert_conductor
. I can uniformise it.
New commits:
add hilbert_ramification
quatalg ramification: discriminant over nf and factor only once
quatalg: add ramified_infinite_places
quatalg: is_division_algebra and is_matrix_ring over nf