Fixes and simplifications for .ramified_primes(), .discriminant() and .is_isomorphic of quaternion algebras
#37164
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Fixed a bug in .discriminant() and .ramified_primes() for rational invariants - Removed unnecessary product and factoring for .ramified_primes() - Reduced description of .ramified_primes() to rational quaternion algebras in preparation of planned .ramified_places() methods over number fields (Amended: Fixed trailing whitespaces)
|
Accidentally closed the pull request due to renaming the branch, so I'll delay the renaming to not open a new PR. |
S17A05
changed the title
.ramified_primes() and .discriminant() of quaternion algebras.ramified_primes(), .discriminant() and .is_isomorphic of quaternion algebras
grhkm21
reviewed
Jan 25, 2024
- Slightly modified description of `.discriminant()` to be more accurate - Cached the method `.ramified_primes()` - Removed optional parameter `sorted` in `.ramified_primes()`, sorting the primes by default instead. - Removed `set()` functions in `.is_isomorphic` since the lists can be compared directly, as they are always sorted according to the above change. - Made description of `.is_isomorphic` slightly more explicit.
|
LGTM, wait for CI |
- Added `.prime_divisors` to import list - Modified product call in `.discriminant()`
|
Documentation preview for this PR (built with commit f5fc230; changes) is ready! 🎉 |
|
I had a look at the failed build & test, and the only issue I could find was related to stale files in sage/tests/books/judson-abstract-algebra; this seems unrelated to this PR, unless there is some connection / a different failure reason that I'm missing. |
|
Yeah ignore those, see devel. That's why I gave |
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Jan 27, 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()` to distinguish between different
formats (prime numbers vs ideals) over Q 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
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
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
.ramified_primes()is_isomorphicto reduce unnecessary calculations.discriminant()and.ramified_primes()where rational invariants caused errors.hilbert_conductorfromsage.arith.miscimport list and added.hilbert_symbol.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:
The original workflow for
.ramified_primes()went as follows:.discriminant(), which calls.hilbert_conductor.hilbert_conductor, the ramified primes are computed and their product (the discriminant of the quaternion algebra) is returnedHence 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 argumentsorted(set toFalseby 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.)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.The function
sage.arith.misc.hilbert_conductorrequires 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.The only call to
sage.arith.misc.hilbert_conductorin quaternion_algebra.py was given in the old computation of the discriminant (the other.hilbert_conductorin the code, also in.discriminant(), refers to the one insage.rings.number_field), so it was removed from the import list. The new approach to.ramified_primes()requiressage.arith.misc.hilbert_symbol, which was added to the import list.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 Implemented.ramified_placesand modified further methods to extend quaternion algebra functionality to number fields #37173)to distinguish between different formats (prime numbers vs ideals) over$\mathbb{Q}$ (Update: this wasn't really feasible, see the issues discussed in QQ.number_field() does not behave like any other NumberField #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