Opened 11 years ago

Closed 11 years ago

Last modified 8 years ago

#6949 closed enhancement (fixed)

add symbolic max and min functions

Reported by: burcin Owned by: burcin
Priority: major Milestone: sage-4.4.2
Component: symbolics Keywords:
Cc: jason Merged in: sage-4.4.2.alpha0
Authors: Burcin Erocal Reviewers: Laurent Fousse
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Matt Riesler wrote on sage-support:

Is it possible to have max behave as you would expect with a symbolic
expression, i.e. wait until you evaluate it or restrict the domain  to
check what is the maximum of the two or more values.

Then kcrisman:

Might there be a way to do something that doesn't conflict with the
builtin max function in the same way as the (nearly reviewed) #3587
seems to avoid conflict with the builtin sum function?  This would be
pretty useful, as currently:

sage: var('x,y')
(x, y)
sage: max(x,y)
x
sage: f(x)=1+x;g(x)=2-x
sage: max(f,g)
x |--> x + 1

which last result is... debatable.

Here is a the thread, which has a simple minded implementation for symbolic max:

http://groups.google.com/group/sage-support/browse_thread/thread/aead15de586984d8

We should make sure the symbolic implementations don't slow down the current builtin python max() and min() too much. If they do, these functions should still be available under a different name.

Attachments (3)

trac_6949-symbolic_min_max.patch (8.3 KB) - added by burcin 11 years ago.
trac_6949-symbolic_min_max.take2.patch (8.5 KB) - added by burcin 11 years ago.
add evalf methods, put common eval and call methods to base class
trac_6949-symbolic_min_max.take3.patch (10.8 KB) - added by burcin 11 years ago.
apply only this patch

Download all attachments as: .zip

Change History (14)

comment:1 Changed 11 years ago by zimmerma

  • Report Upstream set to N/A

see also #8508

comment:2 Changed 11 years ago by burcin

  • Authors set to Burcin Erocal
  • Status changed from new to needs_review

attachment:trac_6949-symbolic_min_max.patch provides basic implementations of max_symbolic() and min_symbolic() functions that work as expected when the input contains symbolic expressions.

The symbolic versions are significantly slower than Python's builtin min() and max(), so replacing the builtin functions with these is not possible at this point.

Changed 11 years ago by burcin

comment:3 Changed 11 years ago by burcin

  • Owner changed from (none) to burcin

I updated attachment:trac_6949-symbolic_min_max.patch to fix a minor doctest error in sage/symbolic/ring.pyx.

comment:4 Changed 11 years ago by jason

  • Cc jason added

comment:5 Changed 11 years ago by lfousse

  • Status changed from needs_review to needs_work

I applied the patch on top of 4.3.5:

sage: f(x) = max_symbolic(sin(x), cos(x))
sage: f(0)
1
sage: f
x |--> max(sin(x), cos(x))
sage: f(1)
max(sin(1), cos(1))

So far, so good. But:

sage: N(f(1))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

/opt/sage-4.3.5/<ipython console> in <module>()

/opt/sage-4.3.5/local/lib/python2.6/site-packages/sage/misc/functional.pyc in numerical_approx(x, prec, digits)
   1161             prec = int((digits+1) * 3.32192) + 1
   1162     try:
-> 1163         return x.numerical_approx(prec)
   1164     except AttributeError:
   1165         from sage.rings.complex_double import is_ComplexDoubleElement

/opt/sage-4.3.5/local/lib/python2.6/site-packages/sage/symbolic/expression.so in sage.symbolic.expression.Expression.n (sage/symbolic/expression.cpp:17043)()

TypeError: cannot evaluate symbolic expression numerically

Changed 11 years ago by burcin

add evalf methods, put common eval and call methods to base class

comment:6 Changed 11 years ago by burcin

  • Status changed from needs_work to needs_review

Thanks for the review.

The new patch attachment:trac_6949-symbolic_min_max.take2.patch replaces the previous one, and implements _evalf_() methods for numeric evaluation. I also created a base class with the common code from __call__() and _eval_() methods.

Can you take another look?

comment:7 Changed 11 years ago by lfousse

Thanks for the updated patch. I applied it on top of 4.4. I can verify the previously reported problem for numerical evaluation works. I have however a new problem:

sage: f(x) = max_symbolic(sin(x), cos(x))
sage: N(integral(f, x, 0, 1))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

/opt/sage-4.4/<ipython console> in <module>()

/opt/sage-4.4/local/lib/python2.6/site-packages/sage/misc/functional.pyc in numerical_approx(x, prec, digits)
   1163             prec = int((digits+1) * 3.32192) + 1
   1164     try:
-> 1165         return x.numerical_approx(prec)
   1166     except AttributeError:
   1167         from sage.rings.complex_double import is_ComplexDoubleElement

/opt/sage-4.4/local/lib/python2.6/site-packages/sage/symbolic/expression.so in sage.symbolic.expression.Expression.n (sage/symbolic/expression.cpp:16928)()

/opt/sage-4.4/local/lib/python2.6/site-packages/sage/symbolic/expression.so in sage.symbolic.expression.Expression._convert (sage/symbolic/expression.cpp:4677)()

/opt/sage-4.4/local/lib/python2.6/site-packages/sage/functions/min_max.pyc in _evalf_(self, *args, **kwds)
    155             TypeError: cannot evaluate symbolic expression numerically
    156         """
--> 157         return max_symbolic(args)
    158
    159 max_symbolic = MaxSymbolic()

/opt/sage-4.4/local/lib/python2.6/site-packages/sage/functions/min_max.pyc in __call__(self, *args, **kwds)
     79                 return args
     80
---> 81         return BuiltinFunction.__call__(self, *args, **kwds)
     82
     83 class MaxSymbolic(MinMax_base):

/opt/sage-4.4/local/lib/python2.6/site-packages/sage/symbolic/function.so in sage.symbolic.function.Function.__call__ (sage/symbolic/function.cpp:4359)()

/opt/sage-4.4/local/lib/python2.6/site-packages/sage/functions/min_max.pyc in _eval_(self, *args)
    136             ValueError: number of arguments must be > 0
    137         """
--> 138         return self.eval_helper(max_symbolic, builtin_max, None, args)
    139
    140     def _evalf_(self, *args, **kwds):

/opt/sage-4.4/local/lib/python2.6/site-packages/sage/functions/min_max.pyc in eval_helper(self, this_f, builtin_f, initial_val, args)
     51
     52         symb_args.append(res)
---> 53         return this_f(*symb_args)
     54
     55     def __call__(self, *args, **kwds):

/opt/sage-4.4/local/lib/python2.6/site-packages/sage/functions/min_max.pyc in __call__(self, *args, **kwds)
     79                 return args
     80
---> 81         return BuiltinFunction.__call__(self, *args, **kwds)
     82
     83 class MaxSymbolic(MinMax_base):

/opt/sage-4.4/local/lib/python2.6/site-packages/sage/symbolic/function.so in sage.symbolic.function.Function.__call__ (sage/symbolic/function.cpp:4192)()

TypeError: cannot coerce arguments: no canonical coercion from <type 'NoneType'> to Symbolic Ring

I don't know if the problem is with integrate or max_symbolic.

Changed 11 years ago by burcin

apply only this patch

comment:8 Changed 11 years ago by burcin

Thanks a lot for testing this. I uploaded a new patch in attachment:trac_6949-symbolic_min_max.take3.patch which should cover many corner cases not handled properly before. The aim is to make {max,min}_symbolic() behave almost identical to the builtin min() and max().

Can you try and break it again? :)

comment:9 Changed 11 years ago by lfousse

  • Status changed from needs_review to positive_review

Applied on top of 4.4.1 this time. I could not break it with my tests, and I also checked -testall, -coverage etc. as per the patch review guidelines. So that would be a positive review. Thanks!

comment:10 Changed 11 years ago by mvngu

  • Merged in set to sage-4.4.2.alpha0
  • Resolution set to fixed
  • Reviewers set to Laurent Fousse
  • Status changed from positive_review to closed

comment:11 Changed 8 years ago by kcrisman

See #13857 for adding this to the reference manual, which somehow never happened but would be worthy considering how many emails we still get about this situation.

Note: See TracTickets for help on using tickets.