Opened 10 years ago

Closed 5 years ago

Last modified 5 years ago

#10133 closed defect (fixed)

Make sin(0), log(1) and similar expressions return Sage integers, not Python ints

Reported by: kcrisman Owned by: burcin
Priority: major Milestone: sage-6.4
Component: symbolics Keywords: type coercion symbolic
Cc: eviatarbach Merged in:
Authors: Jeroen Demeyer Reviewers: Ralf Stephan
Report Upstream: N/A Work issues:
Branch: c8c26e1 (Commits) Commit:
Dependencies: #17130 Stopgaps:

Description (last modified by chapoton)

From this thread:

> > sage: type(log(1))
> > <type 'int'>
> > sage: log(1).n()
> > ---------------------------------------------------------------------------
> > AttributeError: 'int' object has no attribute 'n'
> > sage: Integer(log(1)).n()
> > 0.000000000000000
> > sage: a = Integer(1)
> > sage: a.log()
> > 0
> > sage: type(a.log())
> > <type 'int'>
> > sage: from sage.functions.log import function_log
> > sage: function_log(Integer(1))
> > 0
> > sage: type(function_log(Integer(1)))
> > <type 'int'>
> 
> > Is there any way to get around this in the code, or are we pretty much
> > stuck with this because of how GinacFunctions work?  I'm not 100% sure
> > this is a bug in log; maybe instead we should extend int so that
> 
> This is a bug in the log() function, and any other function which
> returns exact values like 0 or 1. We already work around most cases,
> see lines 720-722 and 736-761 of sage/symbolic/function.pyx.
> 
> The correct fix is to change the corresponding pynac functions to
> coerce the exact value to the parent of the argument before returning
> it. For example, all the lines "return _ex1;" or "return _ex0;" in
> 
> http://pynac.sagemath.org/hg/file/b233d9dadcfa/ginac/inifcns_trans.cpp
> 
> has to be changed this way.
> 

Yup, I see what you are talking about - e.g. 

if (x.is_equal(_ex1))  // log(1) -> 0
     return _ex0;

Although it might be nice to stay relatively close to Ginac and fix such things on the Sage level if that's not really bad.

This ticket's goal is to implement one of these solutions (that is, catch this in function.pyx or in Pynac).

We might as well also deal with sin(0), tan(0), sin(pi) from #10972 here as well, otherwise that really wasn't a duplicate.

Change History (18)

comment:1 Changed 9 years ago by kcrisman

#10972 was closed as a duplicate. Ironic that I reported both, though in my defense, that ticket does list a few specific functions beyond log for which that happens, and it was 8 months ago :)

comment:2 Changed 8 years ago by dsm

ping. I just noticed this in a different context.

comment:3 Changed 8 years ago by kcrisman

  • Description modified (diff)

comment:4 Changed 8 years ago by kcrisman

comment:5 Changed 8 years ago by kcrisman

  • Keywords type coercion symbolic added
  • Summary changed from Make sure log(1) returns an Integer, not an int to Make sin(0), log(1), cos(0), and similar expressions return Sage integers, not symbolic expressions or Python ints

See also this sage-devel thread, where it seems that the consensus is that such things should give Sage Integers, though at #10972 Burcin seems to suggest that symbolic expressions are appropriate.

comment:6 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:7 Changed 7 years ago by eviatarbach

  • Cc eviatarbach added

comment:8 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:9 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:10 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:11 Changed 6 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Dependencies set to #17130

comment:12 Changed 6 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/10133
  • Modified changed from 11/26/14 16:17:23 to 11/26/14 16:17:23

comment:13 Changed 6 years ago by jdemeyer

  • Commit set to c8c26e1ca6195841b5bd1bb900a33eef3bb30b60
  • Status changed from new to needs_review

New commits:

6d10729Simplify numerical evaluation of BuiltinFunctions
b6e1ed4Merge remote-tracking branches 'origin/u/jdemeyer/ticket/17131' and 'origin/u/jdemeyer/ticket/17133' into ticket/17130
382f97aMerge branch 'u/jdemeyer/ticket/17130' of trac.sagemath.org:sage into 6.5beta1
726598917130: reviewer's patch: fix typo
c47dbd4Fix Trac #17328 again in a better way
a486db2Call the factorial() method of an Integer
c8c26e1Convert Ginac function results to the correct parent

comment:14 Changed 6 years ago by jdemeyer

  • Summary changed from Make sin(0), log(1), cos(0), and similar expressions return Sage integers, not symbolic expressions or Python ints to Make sin(0), log(1) and similar expressions return Sage integers, not Python ints

comment:15 Changed 5 years ago by rws

  • Reviewers set to Ralf Stephan
  • Status changed from needs_review to positive_review

Looks good and passes make ptestlong.

comment:16 Changed 5 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/10133 to c8c26e1ca6195841b5bd1bb900a33eef3bb30b60
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:17 Changed 5 years ago by rws

  • Commit c8c26e1ca6195841b5bd1bb900a33eef3bb30b60 deleted

Actually this should have been fixed in Pynac. Now, accidentally the recent patches dealing with https://github.com/pynac/pynac/issues/7 automatically produce this behaviour. Which will be Pynac-0.4.4. The wrapper from this ticket should then be removed.

comment:18 Changed 5 years ago by chapoton

  • Description modified (diff)
Note: See TracTickets for help on using tickets.