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

Allow general base rings for WeierstrassForm #15996

Open
sagetrac-jkeitel mannequin opened this issue Mar 20, 2014 · 6 comments
Open

Allow general base rings for WeierstrassForm #15996

sagetrac-jkeitel mannequin opened this issue Mar 20, 2014 · 6 comments

Comments

@sagetrac-jkeitel
Copy link
Mannequin

sagetrac-jkeitel mannequin commented Mar 20, 2014

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

@sagetrac-jkeitel sagetrac-jkeitel mannequin added this to the sage-6.2 milestone Mar 20, 2014
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 20, 2014

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

3c0999fSmall bugfixes to ensure right coercion.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 20, 2014

Changed commit from b2fb5cc to 3c0999f

@vbraun
Copy link
Member

vbraun commented Mar 25, 2014

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

@sagetrac-jkeitel
Copy link
Mannequin Author

sagetrac-jkeitel mannequin commented Mar 25, 2014

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

@sagetrac-jkeitel

This comment has been minimized.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@jdemeyer
Copy link

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.

@mkoeppe mkoeppe removed this from the sage-6.4 milestone Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants