Skip to content

Commit

Permalink
sagemathgh-38359: Homogenise .log() api across implementations of f…
Browse files Browse the repository at this point in the history
…inite field elements

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Fixes sagemath#38350 by adding `order=` and `check=` arguments to the `.log()`
method of finite field elements, under all implementations.
- `element_pari_ffelt.pyx`: done by @yyyyx4 in sagemath#37329.
- `element_ntl_gf2e.pyx`: if provided, does not compute `base_order`.
- `element_givaro.pyx`: if provided, passes `order` to the underlying
`discrete_log` call.
- `integer_mod.pyx`: the argument is discarded (unless `check=True`, in
which case the order is checked).

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38359
Reported by: Justin Carel
Reviewer(s): Lorenz Panny
  • Loading branch information
Release Manager committed Jul 20, 2024
2 parents df88d06 + 26b8ab2 commit fb0e10a
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 5 deletions.
24 changes: 22 additions & 2 deletions src/sage/rings/finite_rings/element_givaro.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -1411,11 +1411,18 @@ cdef class FiniteField_givaroElement(FinitePolyExtElement):
"""
return Integer(self._cache.log_to_int(self.element))

def log(FiniteField_givaroElement self, base):
def log(FiniteField_givaroElement self, base, order=None, *, check=False):
"""
Return the log to the base `b` of ``self``, i.e., an integer `n`
such that `b^n =` ``self``.
INPUT:
- ``base`` -- non-zero field element
- ``order`` -- integer (optional), multiple of order of ``base``
- ``check`` -- boolean (default: ``False``): If set,
test whether the given ``order`` is correct.
.. WARNING::
TODO -- This is currently implemented by solving the discrete
Expand All @@ -1429,9 +1436,22 @@ cdef class FiniteField_givaroElement(FinitePolyExtElement):
sage: a = b^7
sage: a.log(b)
7
TESTS:
An example for ``check=True``::
sage: F.<t> = GF(3^5, impl='givaro')
sage: t.log(t, 3^4, check=True)
Traceback (most recent call last):
...
ValueError: 81 is not a multiple of the order of the base
"""
b = self.parent()(base)
return sage.groups.generic.discrete_log(self, b)
if (order is not None) and check and not (b**order).is_one():
raise ValueError(f"{order} is not a multiple of the order of the base")

return sage.groups.generic.discrete_log(self, b, ord=order)

def _int_repr(FiniteField_givaroElement self):
r"""
Expand Down
25 changes: 23 additions & 2 deletions src/sage/rings/finite_rings/element_ntl_gf2e.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -1233,14 +1233,17 @@ cdef class FiniteField_ntl_gf2eElement(FinitePolyExtElement):
"""
return unpickleFiniteField_ntl_gf2eElement, (self._parent, str(self))

def log(self, base):
def log(self, base, order=None, *, check=False):
"""
Compute an integer `x` such that `b^x = a`, where `a` is ``self``
and `b` is ``base``.
INPUT:
- ``base`` -- finite field element
- ``order`` -- integer (optional), the order of the base
- ``check`` -- boolean (default: ``False``): If set,
test whether the given ``order`` is correct.
OUTPUT:
Expand Down Expand Up @@ -1278,16 +1281,34 @@ cdef class FiniteField_ntl_gf2eElement(FinitePolyExtElement):
...
ValueError: no logarithm of z50 exists to base z50^49 + z50^46 + z50^45 + z50^44 + z50^41 + z50^34 + z50^33 + z50^32 + z50^27 + z50^25 + z50^24 + z50^21 + z50^18 + z50^17 + z50^16 + z50^15 + z50^12 + z50^11 + z50^10 + z50^8 + z50^7 + z50^3 + z50^2
An example for ``check=True``::
sage: F.<t> = GF(2^5, impl='ntl')
sage: t.log(t, 3^4, check=True)
Traceback (most recent call last):
...
ValueError: base does not have the provided order
AUTHORS:
- David Joyner and William Stein (2005-11)
- Lorenz Panny (2021-11): use PARI's :pari:`fflog` instead of :func:`sage.groups.generic.discrete_log`
"""
cdef Integer base_order

base = self.parent()(base)

# The result is undefined if the input to fflog() is invalid.
# Since the unit group is cyclic, it suffices to check orders.
cdef Integer base_order = base.multiplicative_order()
if order is None:
base_order = base.multiplicative_order()
else:
if check:
from sage.groups.generic import has_order
if not has_order(base, order, '*'):
raise ValueError('base does not have the provided order')
base_order = order

cdef Integer self_order = self.multiplicative_order()
if not self_order.divides(base_order):
raise ValueError(f'no logarithm of {self} exists to base {base}')
Expand Down
23 changes: 22 additions & 1 deletion src/sage/rings/finite_rings/integer_mod.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ cdef class IntegerMod_abstract(FiniteRingElement):
else:
return sib(self.parent())(v)

def log(self, b=None):
def log(self, b=None, order=None, check=False):
r"""
Compute the discrete logarithm of this element to base `b`,
that is,
Expand All @@ -654,6 +654,13 @@ cdef class IntegerMod_abstract(FiniteRingElement):
``R.multiplicative_generator()`` is used, where
``R`` is the parent of ``self``.
- ``order`` -- integer (unused), the order of ``b``.
This argument is normally unused, only there for
coherence of apis with finite field elements.
- ``check`` -- boolean (default: ``False``). If set,
test whether the given ``order`` is correct.
OUTPUT:
Expand Down Expand Up @@ -760,6 +767,15 @@ cdef class IntegerMod_abstract(FiniteRingElement):
sage: R(1).factor()
1
An example for ``check=True``::
sage: F = GF(127, impl='modn')
sage: t = F.primitive_element()
sage: t.log(t, 57, check=True)
Traceback (most recent call last):
...
ValueError: base does not have the provided order
AUTHORS:
- David Joyner and William Stein (2005-11)
Expand All @@ -781,6 +797,11 @@ cdef class IntegerMod_abstract(FiniteRingElement):
if not b.is_unit():
raise ValueError(f"logarithm with base {b} is not defined since it is not a unit modulo {b.modulus()}")

if check:
from sage.groups.generic import has_order
if not has_order(b, order, '*'):
raise ValueError('base does not have the provided order')

cdef Integer n = Integer()
cdef Integer m = one_Z
cdef Integer q, na, nb
Expand Down

0 comments on commit fb0e10a

Please sign in to comment.