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

Implemented .ramified_places and modified further methods to extend quaternion algebra functionality to number fields #37173

Open
wants to merge 31 commits into
base: develop
Choose a base branch
from

Conversation

S17A05
Copy link
Member

@S17A05 S17A05 commented Jan 27, 2024

  1. Implemented method .ramified_places for quaternion algebras over number fields. Integrated .ramified_primes() into it in the process.
  2. Modified .is_division_algebra(), .is_matrix_ring() and .is_isomorphic to use .ramified_places instead of .discriminant(), thus extending them to base number fields.
  3. Rerouted .discriminant() through .ramified_places since the original call to .hilbert_conductor also computed all finite ramified places.
  4. Added .is_totally_definite() and moved is_definite().

Some more detail:

  1. The new method .ramified_places returns all places at which the quaternion algebra self ramifies; this includes the infinite places by default, but can be reduced to only the finite places with the optional parameter inf. The old version of .ramified_primes() from Fixes and simplifications for .ramified_primes(), .discriminant() and .is_isomorphic of quaternion algebras #37164 has been integrated into .ramified_places, thus setting the former up for possible future deprecation; currently it calls self.ramified_places(inf=False) for backwards compatibility.

  2. .is_division_algebra() and .is_matrix_ring() now instead check whether the list of ramified places (finite and infinite) is trivial. .is_isomorphic now compares the set of finite ramified places and, unless working over $\mathbb{Q}$, the list of infinite ramified places of both algebras. The latter can be compared as lists since the real embeddings of the number field are sorted independently of each algebras' invariants, but the former (probably) need to be compared as sets since the order of the list depends on the primes above the respective invariants. The docstring of .is_isomorphic (as well as some of the other docstrings) now includes an example of a non-split quaternion algebra with trivial discriminant, namely the algebra with invariants $(-1,-1)$ over the quadratic field $\mathbb{Q}(\sqrt{3})$.

Possible future work:

…lity to number fields

- Implemented method `.ramified_places` for quaternion algebras over number fields. Integrated `.ramified_primes()` into it in the process
- Rerouted `.discriminant()` through `.ramified_places`
- Modified `.is_division_algebra()`, `.is_matrix_ring()` and `.is_isomorphic` to use `.ramified_places` instead of `.discriminant()`, extending them to base number fields
- Added `.is_definite()` and `.is_totally_definite()` methods
- Added Voight's book "Quaternion Algebras" to the list of references
@S17A05
Copy link
Member Author

S17A05 commented Jan 27, 2024

Fixed some whitespaces and blank lines discovered by lint. Also corrected the formatting of references to Voight's book - thanks to @grhkm21 for pointing this out to me!

@S17A05
Copy link
Member Author

S17A05 commented Jan 28, 2024

After fixing some typos and some errors in the docstrings, I believe this should be ready for review now.

vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 29, 2024
… `.discriminant()` and `.is_isomorphic` of quaternion algebras

    
1. Removed unnecessary product and factorization for
`.ramified_primes()`
2. Adapted `is_isomorphic` to reduce unnecessary calculations
3. Fixed a bug in `.discriminant()` and `.ramified_primes()` where
rational invariants caused errors
4. Removed `.hilbert_conductor` from `sage.arith.misc` import list and
added `.hilbert_symbol`
5. Reduced restriction of `.ramified_primes()` to rational quaternion
algebras in docstring in preparation for planned `.ramified_places()`
function over number fields (currently being worked on)

In more detail:
1. The original workflow for `.ramified_primes()` went as follows:
    - Call `.discriminant()`, which calls `.hilbert_conductor`
    - Inside `.hilbert_conductor`, the ramified primes are computed and
their product (the discriminant of the quaternion algebra) is returned
    - Finally, factor the discriminant back into its prime factors

    Hence we have a redundant product and, more crucially, a redundant
prime factorization. This fix modifies `.ramified_primes()` to instead
directly build the list computed in `.hilbert_conductor()` (up to a bug
fix described in 3.) and return it; the list might not always be sorted
by magnitude of primes, so an optional argument `sorted` (set to `False`
by default) allows to enforce this (small to large) sorting.
Furthermore, `.discriminant()` has been adapted to directly take the
product of the list returned by `.ramified_primes()` (only in the
rational case, for now - see 5.)

2. Since the `.discriminant()`-function needs to compute all (finite)
ramified primes (this was also true before this PR, it was just hidden
inside `.hilbert_conductor()` instead), the function `.is_isomorphic()`
now compares the unsorted lists of finite ramified primes to decide
whether two rational quaternion algebras are isomorphic.

3. The function `sage.arith.misc.hilbert_conductor` requires its
arguments to be integers (to create certain lists of prime divisors);
since it was originally used to determine the discriminant (and, as
explained in 1., the ramified primes), it raises an error when the
invariants are proper rational numbers. To get around the analogous
error for the method `.hilbert_symbol`, we instead look at the
numerators and denominators of both invariants separately, using the
fact that we can (purely on a mathematical level) rescale both
invariants by the squares of their respective denominators without
leaving the isomorphism class of the algebra.

4. The only call to `sage.arith.misc.hilbert_conductor` in
quaternion_algebra.py was given in the old computation of the
discriminant (the other `.hilbert_conductor` in the code, also in
`.discriminant()`, refers to the one in `sage.rings.number_field`), so
it was removed from the import list. The new approach to
`.ramified_primes()` requires `sage.arith.misc.hilbert_symbol`, which
was added to the import list.

5. As of now the `.ramified_primes()`-method is only supported for
rational quaternion algebras. I'm currently working on a version over
number fields, but once it works correctly this will be implemented as a
new function `.ramified_places` (Update: see sagemath#37173) ~~to distinguish
between different formats (prime numbers vs ideals) over $\mathbb{Q}$~~
(Update: this wasn't really feasible, see the issues discussed in sagemath#7596;
thanks to @yyyyx4 for pointing me towards this discussion) ~~and,
furthermore,~~ to not cause confusion using the term "primes" for the
Archimedean real places where a quaternion algebra might ramify. Hence
the implementation restriction in the docstring of `.ramified_primes()`
was removed, but the method still throws a ValueError if not called with
a quaternion algebra defined over the rational numbers.

#sd123
    
URL: sagemath#37164
Reported by: Sebastian Spindler
Reviewer(s): grhkm21, Sebastian Spindler
S17A05 added a commit to jtcc2/sage that referenced this pull request Jan 29, 2024
- Removed greek letter alpha in docstrings in hopes of this fixing infinite build loops
- Updated `.is_definite()` to fit PR sagemath#37173 (up to the reference to Voight's book)
- Other small modifications of docstrings, comments and error warnings
- Slightly cleaned up code with respect to intermediately defined variables
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 30, 2024
… `.discriminant()` and `.is_isomorphic` of quaternion algebras

    
1. Removed unnecessary product and factorization for
`.ramified_primes()`
2. Adapted `is_isomorphic` to reduce unnecessary calculations
3. Fixed a bug in `.discriminant()` and `.ramified_primes()` where
rational invariants caused errors
4. Removed `.hilbert_conductor` from `sage.arith.misc` import list and
added `.hilbert_symbol`
5. Reduced restriction of `.ramified_primes()` to rational quaternion
algebras in docstring in preparation for planned `.ramified_places()`
function over number fields (currently being worked on)

In more detail:
1. The original workflow for `.ramified_primes()` went as follows:
    - Call `.discriminant()`, which calls `.hilbert_conductor`
    - Inside `.hilbert_conductor`, the ramified primes are computed and
their product (the discriminant of the quaternion algebra) is returned
    - Finally, factor the discriminant back into its prime factors

    Hence we have a redundant product and, more crucially, a redundant
prime factorization. This fix modifies `.ramified_primes()` to instead
directly build the list computed in `.hilbert_conductor()` (up to a bug
fix described in 3.) and return it; the list might not always be sorted
by magnitude of primes, so an optional argument `sorted` (set to `False`
by default) allows to enforce this (small to large) sorting.
Furthermore, `.discriminant()` has been adapted to directly take the
product of the list returned by `.ramified_primes()` (only in the
rational case, for now - see 5.)

2. Since the `.discriminant()`-function needs to compute all (finite)
ramified primes (this was also true before this PR, it was just hidden
inside `.hilbert_conductor()` instead), the function `.is_isomorphic()`
now compares the unsorted lists of finite ramified primes to decide
whether two rational quaternion algebras are isomorphic.

3. The function `sage.arith.misc.hilbert_conductor` requires its
arguments to be integers (to create certain lists of prime divisors);
since it was originally used to determine the discriminant (and, as
explained in 1., the ramified primes), it raises an error when the
invariants are proper rational numbers. To get around the analogous
error for the method `.hilbert_symbol`, we instead look at the
numerators and denominators of both invariants separately, using the
fact that we can (purely on a mathematical level) rescale both
invariants by the squares of their respective denominators without
leaving the isomorphism class of the algebra.

4. The only call to `sage.arith.misc.hilbert_conductor` in
quaternion_algebra.py was given in the old computation of the
discriminant (the other `.hilbert_conductor` in the code, also in
`.discriminant()`, refers to the one in `sage.rings.number_field`), so
it was removed from the import list. The new approach to
`.ramified_primes()` requires `sage.arith.misc.hilbert_symbol`, which
was added to the import list.

5. As of now the `.ramified_primes()`-method is only supported for
rational quaternion algebras. I'm currently working on a version over
number fields, but once it works correctly this will be implemented as a
new function `.ramified_places` (Update: see sagemath#37173) ~~to distinguish
between different formats (prime numbers vs ideals) over $\mathbb{Q}$~~
(Update: this wasn't really feasible, see the issues discussed in sagemath#7596;
thanks to @yyyyx4 for pointing me towards this discussion) ~~and,
furthermore,~~ to not cause confusion using the term "primes" for the
Archimedean real places where a quaternion algebra might ramify. Hence
the implementation restriction in the docstring of `.ramified_primes()`
was removed, but the method still throws a ValueError if not called with
a quaternion algebra defined over the rational numbers.

#sd123
    
URL: sagemath#37164
Reported by: Sebastian Spindler
Reviewer(s): grhkm21, Sebastian Spindler
vbraun pushed a commit to vbraun/sage that referenced this pull request Feb 1, 2024
… `.discriminant()` and `.is_isomorphic` of quaternion algebras

    
1. Removed unnecessary product and factorization for
`.ramified_primes()`
2. Adapted `is_isomorphic` to reduce unnecessary calculations
3. Fixed a bug in `.discriminant()` and `.ramified_primes()` where
rational invariants caused errors
4. Removed `.hilbert_conductor` from `sage.arith.misc` import list and
added `.hilbert_symbol`
5. Reduced restriction of `.ramified_primes()` to rational quaternion
algebras in docstring in preparation for planned `.ramified_places()`
function over number fields (currently being worked on)

In more detail:
1. The original workflow for `.ramified_primes()` went as follows:
    - Call `.discriminant()`, which calls `.hilbert_conductor`
    - Inside `.hilbert_conductor`, the ramified primes are computed and
their product (the discriminant of the quaternion algebra) is returned
    - Finally, factor the discriminant back into its prime factors

    Hence we have a redundant product and, more crucially, a redundant
prime factorization. This fix modifies `.ramified_primes()` to instead
directly build the list computed in `.hilbert_conductor()` (up to a bug
fix described in 3.) and return it; the list might not always be sorted
by magnitude of primes, so an optional argument `sorted` (set to `False`
by default) allows to enforce this (small to large) sorting.
Furthermore, `.discriminant()` has been adapted to directly take the
product of the list returned by `.ramified_primes()` (only in the
rational case, for now - see 5.)

2. Since the `.discriminant()`-function needs to compute all (finite)
ramified primes (this was also true before this PR, it was just hidden
inside `.hilbert_conductor()` instead), the function `.is_isomorphic()`
now compares the unsorted lists of finite ramified primes to decide
whether two rational quaternion algebras are isomorphic.

3. The function `sage.arith.misc.hilbert_conductor` requires its
arguments to be integers (to create certain lists of prime divisors);
since it was originally used to determine the discriminant (and, as
explained in 1., the ramified primes), it raises an error when the
invariants are proper rational numbers. To get around the analogous
error for the method `.hilbert_symbol`, we instead look at the
numerators and denominators of both invariants separately, using the
fact that we can (purely on a mathematical level) rescale both
invariants by the squares of their respective denominators without
leaving the isomorphism class of the algebra.

4. The only call to `sage.arith.misc.hilbert_conductor` in
quaternion_algebra.py was given in the old computation of the
discriminant (the other `.hilbert_conductor` in the code, also in
`.discriminant()`, refers to the one in `sage.rings.number_field`), so
it was removed from the import list. The new approach to
`.ramified_primes()` requires `sage.arith.misc.hilbert_symbol`, which
was added to the import list.

5. As of now the `.ramified_primes()`-method is only supported for
rational quaternion algebras. I'm currently working on a version over
number fields, but once it works correctly this will be implemented as a
new function `.ramified_places` (Update: see sagemath#37173) ~~to distinguish
between different formats (prime numbers vs ideals) over $\mathbb{Q}$~~
(Update: this wasn't really feasible, see the issues discussed in sagemath#7596;
thanks to @yyyyx4 for pointing me towards this discussion) ~~and,
furthermore,~~ to not cause confusion using the term "primes" for the
Archimedean real places where a quaternion algebra might ramify. Hence
the implementation restriction in the docstring of `.ramified_primes()`
was removed, but the method still throws a ValueError if not called with
a quaternion algebra defined over the rational numbers.

#sd123
    
URL: sagemath#37164
Reported by: Sebastian Spindler
Reviewer(s): grhkm21, Sebastian Spindler
@yyyyx4 yyyyx4 requested a review from pjbruin February 2, 2024 14:21
S17A05 added a commit to jtcc2/sage that referenced this pull request Feb 3, 2024
- Removed greek letter alpha in docstrings in hopes of this fixing infinite build loops
- Updated `.is_definite()` to fit PR sagemath#37173 (up to the reference to Voight's book)
- Other small modifications of docstrings, comments and error warnings
- Slightly cleaned up code with respect to intermediately defined variables
yyyyx4 pushed a commit to yyyyx4/sage that referenced this pull request Feb 5, 2024
- Removed greek letter alpha in docstrings in hopes of this fixing infinite build loops
- Updated `.is_definite()` to fit PR sagemath#37173 (up to the reference to Voight's book)
- Other small modifications of docstrings, comments and error warnings
- Slightly cleaned up code with respect to intermediately defined variables
@S17A05 S17A05 force-pushed the ramified_places branch 3 times, most recently from 5da4cbf to d63f675 Compare March 26, 2024 13:26
Amend: Even more refactoring of `.is_definite()` and `.is_totally_definite()`
Amend 2: Corrected backtick error
Copy link
Contributor

@grhkm21 grhkm21 left a comment

Choose a reason for hiding this comment

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

some reviews

src/sage/algebras/quatalg/quaternion_algebra.py Outdated Show resolved Hide resolved
src/sage/algebras/quatalg/quaternion_algebra.py Outdated Show resolved Hide resolved
src/sage/algebras/quatalg/quaternion_algebra.py Outdated Show resolved Hide resolved
src/sage/algebras/quatalg/quaternion_algebra.py Outdated Show resolved Hide resolved
src/sage/algebras/quatalg/quaternion_algebra.py Outdated Show resolved Hide resolved
src/sage/algebras/quatalg/quaternion_algebra.py Outdated Show resolved Hide resolved
src/sage/algebras/quatalg/quaternion_algebra.py Outdated Show resolved Hide resolved
src/sage/algebras/quatalg/quaternion_algebra.py Outdated Show resolved Hide resolved
src/sage/algebras/quatalg/quaternion_algebra.py Outdated Show resolved Hide resolved
- Rewrote some docstring descriptions
- Broke apart examples into multiple blocks, with added flavor text
- Modified docstring examples to emphasize certain features more
- Refactored some larger descriptions to use bullet point lists

Amend 1: Fixed missing space before NOTE
Amend 2: Fixed indent of second line in bullet point list in `.discriminant()`
- Added missing check that the base field of a totally definite algebra needs to be totally real
- Moved note in `.ramified_places` below input and moved the interal info on initial choice of finite primes to code comments
vbraun pushed a commit to vbraun/sage that referenced this pull request Mar 30, 2024
…unction fields

    
Implemented the computation of the (local) Hilbert symbol at a place of
a global function field of odd characteristic following formula 12.4.10
in John Voight's "Quaternion Algebras" (in preparation of extending some
quaternion algebra functionality to these global function fields).

The global Hilbert symbol will be implemented via a call to
`.is_matrix_ring()` of the appropriate quaternion algebra once the
latter method has been extended using an extension of PR sagemath#37173, which
will be done once both this PR and the referenced PR have been merged.
    
URL: sagemath#37554
Reported by: Sebastian A. Spindler
Reviewer(s): Kwankyu Lee, Sebastian A. Spindler
vbraun pushed a commit to vbraun/sage that referenced this pull request Mar 30, 2024
    
Updated details for John Voight's book "Quaternion Algebras" in the list
of references and modified some docstrings in `quaternion_algebra.py`.
Split off from sagemath#37173.
    
URL: sagemath#37557
Reported by: Sebastian A. Spindler
Reviewer(s): Travis Scrimshaw
Copy link

github-actions bot commented Apr 6, 2024

Documentation preview for this PR (built with commit 7c04029; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@S17A05
Copy link
Member Author

S17A05 commented Apr 13, 2024

The current test failures seem unrelated (I'm unable to reproduce the failure in graphs.generators.random, though); also Build&Test/build seems to test all files as new, at least on merging the new beta version.

- Avoids possible future issues caused by rounding precision
- Seems to be more efficient

Amend: Docstring fix
@S17A05
Copy link
Member Author

S17A05 commented Jun 25, 2024

There's quite a few failures in the Build & Test using Conda (macos, 3.11), but as far as I can tell they are all not related to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants