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

QuotientRing elements are not callable #11199

Open
vbraun opened this issue Apr 14, 2011 · 8 comments
Open

QuotientRing elements are not callable #11199

vbraun opened this issue Apr 14, 2011 · 8 comments

Comments

@vbraun
Copy link
Member

vbraun commented Apr 14, 2011

It would be convenient if quotient ring elements would be callable like polynomials. This is currently not the case:

sage: R.<x,y> = QQ[]
sage: I = R.ideal(x)
sage: Q = R.quotient_ring(I)
sage: Q
Quotient of Multivariate Polynomial Ring in x, y over Rational Field by the ideal (x)
sage: Q.inject_variables()
Defining xbar, ybar
sage: p = xbar + ybar
sage: p(1,1)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

/home/vbraun/opt/sage-4.7.alpha4/devel/sage-main/<ipython console> in <module>()

TypeError: 'QuotientRingElement' object is not callable

The attached patch implements this functionality by handing the argument through to the lift.

Arguably, it should check that the result does not depend on the presentation. Pro: catches errors; Con: potentially slow, ideal membership test necessary if arguments are polynomials.


Apply attachment: trac_11199-jhp.patch.

CC: @novoselt

Component: algebra

Author: Volker Braun

Reviewer: John Palmieri

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

@vbraun vbraun added this to the sage-5.11 milestone Apr 14, 2011
@vbraun
Copy link
Member Author

vbraun commented Apr 14, 2011

Initial patch

@vbraun
Copy link
Member Author

vbraun commented Apr 14, 2011

comment:1

Attachment: trac_11199_QuotientRingElement_call.patch.gz

@jhpalmieri

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:3

Evaluating an element of a quotient ring doesn't make mathematical sense in general, only if it makes sense to evaluate the lift: if you're taking a quotient of a group algebra or the p-adics, what does it mean to evaluate an element of it? I'm attaching a new patch which adds a comment about this to the docstring; is this enough warning?

I'm happy with the original patch as far as it goes, and I'm almost ready to give it a positive review, but I'd like to hear opinions about the issue I raised.

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@jhpalmieri
Copy link
Member

Attachment: trac_11199-delta.patch.gz

for reference only; difference beteen initial patch and jhp patch

@jhpalmieri
Copy link
Member

apply this one

@sagetrac-fwclarke
Copy link
Mannequin

sagetrac-fwclarke mannequin commented Dec 18, 2011

comment:4

Attachment: trac_11199-jhp.patch.gz

Even with the extra warning in the docstring, users could very easily be misled.
For to make the evaluation independent of the lift it is necessary
for the ideal defining the quotient to annihilate the argument.

I think it would be sensible to have a parameter 'check' with default True, and a
clause like:

if check and any(g(args) !=0 for g in f.parent().defining_ideal().gens()):
    raise ValueError, "argument not in the zero set of the ideal"

That way an unthinking user is not going to be caught out, while someone who
knows what they're doing can speed things up by setting 'check=False'.

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@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
@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

5 participants