sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.46k stars 484 forks source link

Allow general base rings for WeierstrassForm #15996

Open a1031e5c-5e2c-4c90-8ba4-91be67e304df opened 10 years ago

a1031e5c-5e2c-4c90-8ba4-91be67e304df commented 10 years ago

Currently, one method in sage.rings.invariant_theory and another one in sage.schemes.toric.weierstrass make use of __floordiv__. More precisely, if p and m are elements in a ring R with base ring B, then one needs p // m, where m always has a unit coefficient. However, __floordiv__ is only implemented if B is a field and therefore doing something like

sage: P.<a, b> = QQ[]
sage: R.<x,y,z> = P[]
sage: cubic = x^3 + a*y^3 + a^2*z^3
sage: WeierstrassForm(cubic)

does not work because

sage: cubic // x^3

fails. However, since the coefficients of m are always in QQ, we can work around that and I've written a short patch that does so.

It might not be very pretty (if someone has a nicer idea, that would be great), but it works and actually speeds up long calculations.

Best, Jan

CC: @vbraun

Component: algebraic geometry

Keywords: toric, weierstrass

Author: Jan Keitel

Branch/Commit: u/jkeitel/weierstrass_general_rings @ 3c0999f

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

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

3c0999fSmall bugfixes to ensure right coercion.
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from b2fb5cc to 3c0999f

vbraun commented 10 years ago
comment:3

The floor division works in 6.2.beta5, possibly due to #13048:

sage: P.<a> = QQ[]
sage: R.<x,y,z> = P[]
sage: cubic = x^3 + a*y^3 + a^2*z^3
sage: cubic // x^3
1
a1031e5c-5e2c-4c90-8ba4-91be67e304df commented 10 years ago
comment:4

Hi Volker,

without the patch, the following still does not work:

sage: P.<a,b> = QQ[]
sage: R.<x,y,z> = P[]
sage: cubic = x^3 + a*y^3 + a^2*z^3
sage: cubic // x^3
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-11-bf6b2864fe25> in <module>()
----> 1 cubic // x**Integer(3)

/home/pcl337b/jkeitel/sage/sage/local/lib/python2.7/site-packages/sage/rings/polynomial/multi_polynomial_element.pyc in __floordiv__(self, right)
   1416             for c,m in self:
   1417                 t = c*m
-> 1418                 if denC.divides(c) and P.monomial_divides(denM, m):
   1419                     ret += P.monomial_quotient(t, right, coeff=True)
   1420             return ret

/home/pcl337b/jkeitel/sage/sage/local/lib/python2.7/site-packages/sage/structure/parent.so in sage.structure.parent.Parent.__getattr__ (sage/structure/parent.c:7221)()

/home/pcl337b/jkeitel/sage/sage/local/lib/python2.7/site-packages/sage/structure/misc.so in sage.structure.misc.getattr_from_other_class (sage/structure/misc.c:1687)()

AttributeError: 'MPolynomialRing_polydict_with_category' object has no attribute 'monomial_divides'

Unfortunately, I don't know enough about the internals of (multivariate) rings in Sage. Is there a better cast than the one in #13048 that one could make to get this to work, too?

Best, Jan

a1031e5c-5e2c-4c90-8ba4-91be67e304df commented 10 years ago

Description changed:

--- 
+++ 
@@ -3,7 +3,7 @@
 where `m` always has a unit coefficient. However, `__floordiv__` is only implemented if B is a field and therefore doing something like

-sage: P. = QQ[] +sage: P.<a, b> = QQ[] sage: R.<x,y,z> = P[] sage: cubic = x^3 + ay^3 + a^2z^3 sage: WeierstrassForm(cubic)

jdemeyer commented 8 years ago
comment:8

This description is way too vague to understand what _fdiv does:

def _fdiv(a, b):
    r"""
    Return a // b even if b is an element of a ring 
    whose base ring is not a field.

    This is just a helper function to allow divisions a // b
    for a and b elements of a ring over another ring.
    It works only if b has constant coefficients.