Opened 7 years ago

Closed 7 years ago

#553 closed enhancement (fixed)

[with patch, positive review] calling of symbolic expressions is sometimes ridiculous

Reported by: was Owned by: mhansen
Priority: major Milestone: sage-2.9
Component: calculus Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

The input should be

       f = x^(1/9) + (2^(8/9) - 2^(1/9))*(x - 1) - x^(8/9)

Note that * before (x-1).  That your input was accepted is an indication
that SAGE should be more restrictive with what it allows.  What's
happening is that (2^(8/9) - 2^(1/9)) is parsed as a symbolic expression (a
constant function), and then 2^(8/9) - 2^(1/9))(x - 1) is the value of that
constant function at x-1.  Yep, that this is allowed is ridiculous, and should
be changed (I've filed a bug report). 

Attachments (3)

553.patch (2.0 KB) - added by mhansen 7 years ago.
533.patch (6.5 KB) - added by mhansen 7 years ago.
trac-553-part2.patch (1.8 KB) - added by was 7 years ago.
apply this patch and the one right above it.

Download all attachments as: .zip

Change History (13)

Changed 7 years ago by mhansen

comment:1 Changed 7 years ago by mhansen

  • Owner changed from was to mhansen

I attached a patch which throws an error when trying to substitute into a constant expression.
It can be overridden by passing a substitute=True parameter to call.

All doc tests pass with it.

comment:2 Changed 7 years ago by was

  • Resolution set to fixed
  • Status changed from new to closed

comment:3 Changed 7 years ago by was

  • Resolution fixed deleted
  • Status changed from closed to reopened

This patch doesn't fix the problem at all. If you apply it the above example
doesn't raise an error. In fact, it's a patch against named substitutions, which
should *always* be allowed. So this was totally wrong.

I'm worried actually that the expense of determining whether or not an expression
is too costly. The only reasonable fix is to completely ban calling "non-callable"
symbolic expressions without an explicit substitution. doing this is a lot more
work though.

comment:4 Changed 7 years ago by was

  • Milestone changed from sage-2.9 to sage-3
  • Type changed from defect to enhancement

comment:5 Changed 7 years ago by mabshoff

  • Milestone changed from sage-feature to sage-2.8.13
  • Summary changed from calling of symbolic expressions is sometimes ridiculous to [with patch] calling of symbolic expressions is sometimes ridiculous

comment:6 Changed 7 years ago by mabshoff

  • Summary changed from [with patch] calling of symbolic expressions is sometimes ridiculous to [with broken patch] calling of symbolic expressions is sometimes ridiculous

comment:7 Changed 7 years ago by mhansen

I think the best way to go about doing this is to add a number_of_arguments() to SymbolicExpressions?:

sage: sin.number_of_arguments()
1
sage: (sin+1).number_of_arguments()
1
sage: (sin+x).number_of_arguments()
1
sage: (sin+x+y).number_of_arguments()
2
sage: (2^(8/9)-2^(1/9)).number_of_arguments()
0

Changed 7 years ago by mhansen

comment:8 Changed 7 years ago by mhansen

  • Summary changed from [with broken patch] calling of symbolic expressions is sometimes ridiculous to [with patch] calling of symbolic expressions is sometimes ridiculous

I've put up a new patch.

Changed 7 years ago by was

apply this patch and the one right above it.

comment:9 Changed 7 years ago by was

  • Summary changed from [with patch] calling of symbolic expressions is sometimes ridiculous to [with patch, positive review] calling of symbolic expressions is sometimes ridiculous

comment:10 Changed 7 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from reopened to closed

Merged in 2.9.rc0.

Note: See TracTickets for help on using tickets.