sagemath / sage

Main repository of SageMath. Now open for Issues and Pull Requests.
https://www.sagemath.org
Other
1.2k stars 413 forks source link

ramification properties of quaternion algebras #22494

Open cd1fd1a3-1fd5-4c80-88ce-04d187901b3d opened 7 years ago

cd1fd1a3-1fd5-4c80-88ce-04d187901b3d commented 7 years ago

Improvements:

CC: @adeines @tornaria @mmasdeu

Component: number theory

Keywords: days84, quaternion algebras, hilbert symbol, ramification

Author: Aurel Page

Branch/Commit: u/aurel/quatalg_ramification @ a828d9e

Issue created by migration from https://trac.sagemath.org/ticket/22494

cd1fd1a3-1fd5-4c80-88ce-04d187901b3d commented 7 years ago

Branch: u/aurel/quatalg_ramification

cd1fd1a3-1fd5-4c80-88ce-04d187901b3d commented 7 years ago

Commit: a828d9e

cd1fd1a3-1fd5-4c80-88ce-04d187901b3d commented 7 years ago

New commits:

ecc7ac8add hilbert_ramification
1c421f8quatalg ramification: discriminant over nf and factor only once
0fd6575quatalg: add ramified_infinite_places
a828d9equatalg: is_division_algebra and is_matrix_ring over nf
cd1fd1a3-1fd5-4c80-88ce-04d187901b3d commented 7 years ago
comment:3

I am not sure about the list vs set choice in ramified_primes and ramified_infinite_places. Tell me what I should do!

cd1fd1a3-1fd5-4c80-88ce-04d187901b3d commented 7 years ago

Changed keywords from quaternion algebras, hilbert symbol, ramification to days84, quaternion algebras, hilbert symbol, ramification

videlec commented 7 years ago
comment:5

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?

cd1fd1a3-1fd5-4c80-88ce-04d187901b3d commented 7 years ago
comment:6

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.