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

deprecate constructing number-field fractional ideals via orders' .ideal() method #34979

Merged
merged 1 commit into from
Mar 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 18 additions & 14 deletions src/sage/matrix/matrix2.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,13 @@ from sage.matrix.matrix_misc import permanental_minor_polynomial
# used to deprecate only adjoint method
from sage.misc.superseded import deprecated_function_alias

# temporary hack to silence the warnings from #34806
from sage.rings.number_field.order import Order as NumberFieldOrder
def ideal_or_fractional(R, *args):
if isinstance(R, NumberFieldOrder):
R = R.number_field()
return R.ideal(*args)

_Fields = Fields()

cdef class Matrix(Matrix1):
Expand Down Expand Up @@ -15993,7 +16000,7 @@ cdef class Matrix(Matrix1):
try:
for i in xrange(1, len(pivs)):
y = a[i][pivs[i]]
I = R.ideal(y)
I = ideal_or_fractional(R, y)
s = a[0][pivs[i]]
t = I.small_residue(s)
v = R( (s-t) / y)
Expand Down Expand Up @@ -17730,9 +17737,9 @@ def _smith_diag(d, transformation=True):
else:
left = right = None
for i in xrange(n):
I = R.ideal(dp[i,i])
I = ideal_or_fractional(R, dp[i,i])

if I == R.unit_ideal():
if I == ideal_or_fractional(R, 1):
if dp[i,i] != 1:
if transformation:
left.add_multiple_of_row(i,i,R(R(1)/(dp[i,i])) - 1)
Expand All @@ -17741,12 +17748,12 @@ def _smith_diag(d, transformation=True):

for j in xrange(i+1,n):
if dp[j,j] not in I:
t = R.ideal([dp[i,i], dp[j,j]]).gens_reduced()
t = ideal_or_fractional(R, [dp[i,i], dp[j,j]]).gens_reduced()
if len(t) > 1:
raise ArithmeticError
t = t[0]
# find lambda, mu such that lambda*d[i,i] + mu*d[j,j] = t
lamb = R(dp[i,i]/t).inverse_mod( R.ideal(dp[j,j]/t))
lamb = R(dp[i,i]/t).inverse_mod( ideal_or_fractional(R, dp[j,j]/t))
mu = R((t - lamb*dp[i,i]) / dp[j,j])

newlmat = dp.new_matrix(dp.nrows(), dp.nrows(), 1)
Expand Down Expand Up @@ -17821,18 +17828,15 @@ def _generic_clear_column(m):
# [e,f]
# is invertible over R

if a[0,0] != 0:
I = R.ideal(a[0, 0]) # need to make sure we change this when a[0,0] changes
else:
I = R.zero_ideal()
I = ideal_or_fractional(R, a[0, 0]) # need to make sure we change this when a[0,0] changes
for k in xrange(1, a.nrows()):
if a[k,0] not in I:
try:
v = R.ideal(a[0,0], a[k,0]).gens_reduced()
v = ideal_or_fractional(R, a[0,0], a[k,0]).gens_reduced()
except Exception as msg:
raise ArithmeticError("%s\nCan't create ideal on %s and %s" % (msg, a[0,0], a[k,0]))
if len(v) > 1:
raise ArithmeticError("Ideal %s not principal" % R.ideal(a[0,0], a[k,0]))
raise ArithmeticError("Ideal %s not principal" % ideal_or_fractional(R, a[0,0], a[k,0]))
B = v[0]

# now we find c,d, using the fact that c * (a_{0,0}/B) - d *
Expand All @@ -17841,7 +17845,7 @@ def _generic_clear_column(m):
# need to handle carefully the case when a_{k,0}/B is a unit, i.e. a_{k,0} divides
# a_{0,0}.

c = R(a[0,0] / B).inverse_mod(R.ideal(a[k,0] / B))
c = R(a[0,0] / B).inverse_mod(ideal_or_fractional(R, a[k,0] / B))
d = R( (c*a[0,0] - B)/(a[k,0]) )

# sanity check
Expand All @@ -17850,7 +17854,7 @@ def _generic_clear_column(m):

# now we find e,f such that e*d + c*f = 1 in the same way
if c != 0:
e = d.inverse_mod( R.ideal(c) )
e = d.inverse_mod( ideal_or_fractional(R, c) )
f = R((1 - d*e)/c)
else:
e = R(-a[k,0]/B) # here d is a unit and this is just 1/d
Expand All @@ -17866,7 +17870,7 @@ def _generic_clear_column(m):
if newlmat.det() != 1:
raise ArithmeticError
a = newlmat*a
I = R.ideal(a[0,0])
I = ideal_or_fractional(R, a[0,0])
left_mat = newlmat*left_mat
if left_mat * m != a:
raise ArithmeticError
Expand Down
1 change: 1 addition & 0 deletions src/sage/modular/dirichlet.py
Original file line number Diff line number Diff line change
Expand Up @@ -2339,6 +2339,7 @@ class DirichletGroupFactory(UniqueFactory):
sage: parent(val)
Gaussian Integers in Cyclotomic Field of order 4 and degree 2
sage: r4.residue_field(r4.ideal(29).factor()[0][0])(val)
doctest:warning ... DeprecationWarning: ...
17
sage: r4.residue_field(r4.ideal(29).factor()[0][0])(val) * GF(29)(3)
22
Expand Down
3 changes: 1 addition & 2 deletions src/sage/rings/number_field/bdd_height.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,6 @@ def bdd_height(K, height_bound, tolerance=1e-2, precision=53):
if B < 1:
return
embeddings = K.places(prec=precision)
O_K = K.ring_of_integers()
r1, r2 = K.signature()
r = r1 + r2 - 1
RF = RealField(precision)
Expand Down Expand Up @@ -486,7 +485,7 @@ def log_height_for_generators_approx(alpha, beta, Lambda):
Return a lambda approximation h_K(alpha/beta)
"""
delta = Lambda / (r + 2)
norm_log = delta_approximation(RR(O_K.ideal(alpha, beta).norm()).log(), delta)
norm_log = delta_approximation(RR(K.ideal(alpha, beta).norm()).log(), delta)
log_ga = vector_delta_approximation(log_map(alpha), delta)
log_gb = vector_delta_approximation(log_map(beta), delta)
arch_sum = sum([max(log_ga[k], log_gb[k]) for k in range(r + 1)])
Expand Down
2 changes: 1 addition & 1 deletion src/sage/rings/number_field/number_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -12534,7 +12534,7 @@ def map_Zmstar_to_Zm(h):
p = u.lift()
while not p.is_prime():
p += m
f = R.ideal(p).prime_factors()[0].residue_class_degree()
f = K.fractional_ideal(p).prime_factors()[0].residue_class_degree()
h = g**f
if h not in H:
Hgens += [h]
Expand Down
34 changes: 30 additions & 4 deletions src/sage/rings/number_field/number_field_element.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,8 @@ def _inverse_mod_generic(elt, I):
"""
from sage.matrix.constructor import matrix
R = elt.parent()
try:
I = R.ideal(I)
except ValueError:
I = R.number_field().fractional_ideal(I)
if not I.is_integral():
raise ValueError("inverse is only defined modulo integral ideals")
if I == 0:
raise ValueError("inverse is not defined modulo the zero ideal")
Expand Down Expand Up @@ -1987,6 +1986,9 @@ cdef class NumberFieldElement(FieldElement):
raise ArithmeticError("factorization of 0 is not defined")

K = self.parent()
from .order import is_NumberFieldOrder
if is_NumberFieldOrder(K):
K = K.number_field()
fac = K.ideal(self).factor()
# Check whether all prime ideals in `fac` are principal
for P,e in fac:
Expand All @@ -1999,6 +2001,29 @@ cdef class NumberFieldElement(FieldElement):
from sage.structure.all import Factorization
return Factorization(element_fac, unit=self/element_product)

def is_prime(self):
r"""
Test whether this number-field element is prime as
an algebraic integer.

Note that the behavior of this method differs from the behavior
of :meth:`~sage.structure.element.RingElement.is_prime` in a
general ring, according to which (number) fields would have no
nonzero prime elements.

EXAMPLES::

sage: K.<i> = NumberField(x^2+1)
sage: (1+i).is_prime()
True
sage: ((1+i)/2).is_prime()
False
"""
if not self or not self.is_integral():
return False
I = self.number_field().fractional_ideal(self)
return I.is_prime()

@coerce_binop
def gcd(self, other):
"""
Expand Down Expand Up @@ -2068,7 +2093,8 @@ cdef class NumberFieldElement(FieldElement):
if not is_NumberFieldOrder(R) or not R.is_maximal():
raise NotImplementedError("gcd() for %r is not implemented" % R)

g = R.ideal(self, other).gens_reduced()
K = R.number_field()
g = K.fractional_ideal(self, other).gens_reduced()
if len(g) > 1:
raise ArithmeticError("ideal (%r, %r) is not principal, gcd is not defined" % (self, other) )

Expand Down
8 changes: 8 additions & 0 deletions src/sage/rings/number_field/order.py
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,14 @@ def ideal(self, *args, **kwds):
"""
if not self.is_maximal():
raise NotImplementedError("ideals of non-maximal orders not yet supported.")
from sage.misc.superseded import deprecation
deprecation(34806, 'In the future, constructing an ideal of the ring of '
'integers of a number field will use an implementation '
'compatible with ideals of other (non-maximal) orders, '
'rather than returning an integral fractional ideal of '
'its containing number field. Use .fractional_ideal(), '
'together with an .is_integral() check if desired, to '
'avoid your code breaking with future changes to Sage.')
I = self.number_field().ideal(*args, **kwds)
if not I.is_integral():
raise ValueError("ideal must be integral; use fractional_ideal to create a non-integral ideal.")
Expand Down
2 changes: 1 addition & 1 deletion src/sage/rings/padics/padic_valuation.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ def create_key_and_extra_args_for_number_field_from_ideal(self, R, I, prime):

EXAMPLES::

sage: GaussianIntegers().valuation(GaussianIntegers().ideal(2)) # indirect doctest
sage: GaussianIntegers().valuation(GaussianIntegers().number_field().fractional_ideal(2)) # indirect doctest
2-adic valuation

TESTS:
Expand Down
7 changes: 5 additions & 2 deletions src/sage/rings/polynomial/polynomial_element.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ from sage.categories.morphism cimport Morphism
from sage.misc.superseded import deprecation_cython as deprecation, deprecated_function_alias
from sage.misc.cachefunc import cached_method

from sage.rings.number_field.order import is_NumberFieldOrder
from sage.rings.number_field.order import is_NumberFieldOrder, Order as NumberFieldOrder
from sage.categories.number_fields import NumberFields


Expand Down Expand Up @@ -5105,8 +5105,11 @@ cdef class Polynomial(CommutativeAlgebraElement):
y = self._parent.quo(self).gen()
from sage.groups.generic import order_from_multiple
return n == order_from_multiple(y, n, n_prime_divs, operation="*")
elif isinstance(R, NumberFieldOrder):
K = R.number_field()
return K.fractional_ideal(self.coefficients()) == K.fractional_ideal(1)
else:
return R.ideal(self.coefficients())==R.ideal(1)
return R.ideal(self.coefficients()) == R.ideal(1)

def is_constant(self):
"""
Expand Down
16 changes: 14 additions & 2 deletions src/sage/rings/quotient_ring.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,13 @@ def QuotientRing(R, I, names=None, **kwds):
from sage.rings.polynomial.polynomial_ring_constructor import BooleanPolynomialRing_constructor as BooleanPolynomialRing
kwds.pop('implementation')
return BooleanPolynomialRing(R.ngens(), names=names, **kwds)
if not isinstance(I, ideal.Ideal_generic) or I.ring() != R:
# workaround to silence warning from #34806
from sage.rings.number_field.order import Order
if isinstance(R, Order):
if not R.is_maximal():
raise NotImplementedError('only implemented for maximal orders')
I = R.number_field().ideal(I)
elif not isinstance(I, ideal.Ideal_generic) or I.ring() != R:
I = R.ideal(I)
if I.is_zero():
return R
Expand Down Expand Up @@ -444,7 +450,13 @@ def __init__(self, R, I, names, category=None):
"""
if R not in _Rings:
raise TypeError("The first argument must be a ring, but %s is not"%R)
if I not in R.ideal_monoid():
# workaround to silence warning from #34806
from sage.rings.number_field.order import Order
if isinstance(R, Order):
M = R.number_field().ideal_monoid()
else:
M = R.ideal_monoid()
if I not in M:
raise TypeError("The second argument must be an ideal of the given ring, but %s is not"%I)
self.__R = R
self.__I = I
Expand Down
5 changes: 3 additions & 2 deletions src/sage/schemes/elliptic_curves/ell_number_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -1499,10 +1499,11 @@ def conductor(self):
# Note: for number fields other than QQ we could initialize
# N=K.ideal(1) or N=OK.ideal(1), which are the same, but for
# K == QQ it has to be ZZ.ideal(1).
OK = self.base_ring().ring_of_integers()
K = self.base_field()
N = ZZ.ideal(1) if K is QQ else K.fractional_ideal(1)
self._conductor = prod([d.prime()**d.conductor_valuation()
for d in self.local_data()],
OK.ideal(1))
N)
return self._conductor

def minimal_discriminant_ideal(self):
Expand Down
2 changes: 1 addition & 1 deletion src/sage/schemes/elliptic_curves/heegner.py
Original file line number Diff line number Diff line change
Expand Up @@ -1657,7 +1657,7 @@ def ideal(self):
(A,B,C) = f
if A%c == 0:
A, C = C, A
return K.maximal_order().ideal([A, (-B+c*sqrtD)/2])
return K.fractional_ideal([A, (-B+c*sqrtD)/2])

## def __call__(self, z):
## """
Expand Down
4 changes: 3 additions & 1 deletion src/sage/schemes/projective/projective_point.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
from sage.categories.number_fields import NumberFields
_NumberFields = NumberFields()
from sage.rings.fraction_field import FractionField
from sage.rings.number_field.order import is_NumberFieldOrder
from sage.rings.number_field.order import is_NumberFieldOrder, Order as NumberFieldOrder
from sage.rings.qqbar import number_field_elements_from_algebraics
from sage.rings.quotient_ring import QuotientRing_generic
from sage.rings.rational_field import QQ
Expand Down Expand Up @@ -750,6 +750,8 @@ def global_height(self, prec=None):
raise TypeError("must be defined over an algebraic field")
else:
K = P.codomain().base_ring()
if isinstance(K, NumberFieldOrder):
K = K.number_field()
# first get rid of the denominators
denom = lcm([xi.denominator() for xi in P])
x = [xi * denom for xi in P]
Expand Down