Opened 3 years ago

Closed 3 years ago

#25460 closed defect (fixed)

Add .is_square() function for symbolic expression

Reported by: vklein Owned by: vklein
Priority: major Milestone: sage-8.3
Component: symbolics Keywords:
Cc: vdelecroix Merged in:
Authors: Vincent Klein Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: 88a1145 (Commits, GitHub, GitLab) Commit: 88a1145ddb919216abed6b366b82016c9bd3df43
Dependencies: Stopgaps:

Status badges

Description

Fix the following behaviour :

sage: f(n)=n^2
sage: f(2).is_square()
---------------------------------------------------------------------------
NotImplementedError                       Traceback (most recent call last)
<ipython-input-2-07de71372bf0> in <module>()
----> 1 f(Integer(2)).is_square()

/home/vklein/odk/sage/local/lib/python2.7/site-packages/sage/structure/element.pyx in sage.structure.element.CommutativeRingElement.is_square (build/cythonized/sage/structure/element.c:20900)()
   3020             framework.
   3021         """
-> 3022         raise NotImplementedError("is_square() not implemented for elements of %s" % self.parent())
   3023 
   3024     def sqrt(self, extend=True, all=False, name=None):

NotImplementedError: is_square() not implemented for elements of Symbolic Ring

Change History (17)

comment:1 Changed 3 years ago by vklein

  • Owner changed from (none) to vklein

comment:2 Changed 3 years ago by vklein

  • Branch set to u/vklein/add__is_square___function_for_symbolic_expression

comment:3 Changed 3 years ago by vklein

  • Authors set to Vincent Klein
  • Commit set to ac0dc8ee6ddd9ea9b51785252fa64865ddd172fa
  • Status changed from new to needs_review

New commits:

ac0dc8eTrac #25460: Add is_square method to symbolic expression

comment:4 Changed 3 years ago by vklein

  • Status changed from needs_review to needs_work

Currently is_square((x-1)^2) return True the last commit don't do that.

comment:5 Changed 3 years ago by git

  • Commit changed from ac0dc8ee6ddd9ea9b51785252fa64865ddd172fa to d3fdcae3e988bc2184b77763bfe932226ba04f89

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

d3fdcaeTrac #25460: enable is_square to manage non numeric ...

comment:6 Changed 3 years ago by git

  • Commit changed from d3fdcae3e988bc2184b77763bfe932226ba04f89 to c1c77e73b53bd40927fd3e188c93b2fb39cb3d1e

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

c1c77e7Trac #25460: enable is_square to manage non numeric ...

comment:7 Changed 3 years ago by vklein

  • Status changed from needs_work to needs_review

Enable is_square to manage non numeric expression.

comment:8 Changed 3 years ago by vdelecroix

Note that is_square is completely broken for symbolic expression. Moving it as a method is not helping in any way.

sage: x = SR.var('x')
sage: is_square((cos(x) + 1)^2)
True
sage: is_square((cos(x) + 1)^2, True)   # WTF?
Traceback (most recetn call last):
...
TypeError: unable to convert 2 - 1/2*x^2 + 1/24*x^4 - 1/720*x^6 + 1/40320*x^8 - 1/3628800*x^10 + 1/479001600*x^12 - 1/87178291200*x^14 + 1/20922789888000*x^16 + O(x^18) to a symbolic expression

And for example, the following should be False

sage: is_square(cos(x) - cos(x).taylor(x, 0, 20))
True

I would suggest to let this method raise NotImplementedError whenever it is not possible to call is_square on the underlying pyobject.

comment:9 follow-up: Changed 3 years ago by vklein

This mean is_square((x-1)^2) (with #24677) will have a different behaviour, are you ok with that ?

Last edited 3 years ago by vklein (previous) (diff)

comment:10 in reply to: ↑ 9 Changed 3 years ago by vdelecroix

Replying to vklein:

This mean is_square((x-1)^2) (with #24677) will have a different behaviour, are you ok with that ?

yes

comment:11 Changed 3 years ago by git

  • Commit changed from c1c77e73b53bd40927fd3e188c93b2fb39cb3d1e to db9019418ecc75b5a0e6f9d5f58e0e3bc2a32b30

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

db90194Trac #25460: Raise NotImplementedError when is_square ...

comment:12 Changed 3 years ago by vklein

Done

comment:13 Changed 3 years ago by vdelecroix

try:
    obj = self.pyobject()
except TypeError:
    raise NotImplementedError("is_square() not implemented for non numeric elements of Symbolic Ring")

return obj.is_square()

comment:14 Changed 3 years ago by git

  • Commit changed from db9019418ecc75b5a0e6f9d5f58e0e3bc2a32b30 to 88a1145ddb919216abed6b366b82016c9bd3df43

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

88a1145Trac #25460: Raise NotImplementedError when is_square ...

comment:15 Changed 3 years ago by vklein

Ok

comment:16 Changed 3 years ago by vdelecroix

  • Reviewers set to Vincent Delecroix
  • Status changed from needs_review to positive_review

comment:17 Changed 3 years ago by vbraun

  • Branch changed from u/vklein/add__is_square___function_for_symbolic_expression to 88a1145ddb919216abed6b366b82016c9bd3df43
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.