sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.47k stars 488 forks source link

Refactoring in Chebyshev functions #24554

Open rwst opened 6 years ago

rwst commented 6 years ago

While other orthogonal polynomial functions (as well as almost all other symbolic functions) are simply BuiltinFunctions or GinacFunctions this ticket suffers from the fact that the Cheby functions in Sage allow the algorithm keyword (by using a __call__ method in the superclass). The way such polymorphisms are resolved in other functions is by having the interface (the callable global function chebyshev_T()) dispatching on the keyword, especially because doctests involving all kind of Sage objects as function argument are expected to work eternally.

The goal of this ticket is to move the __call__ method out of the Function class (as well as other methods used as interface) into a separate interface class; to 2. make the remaining class inherit from BuiltinFunction and, 3., to remove the ChebyshevFunction superclass as it is then no longer used.

Component: symbolics

Author: Ralf Stephan

Branch/Commit: u/rws/24554 @ 957ce14

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

rwst commented 6 years ago

Description changed:

--- 
+++ 
@@ -1 +1 @@
-While other orthogonal polynomial functions (as well as almost all other symbolic functions) are simply `BuiltinFunction`s or `GinacFunction`s this ticket suffers from the fact that the Cheby functions in Sage allow the `algorithm` keyword (by using a `__call__` method in the superclass). The way such polymorphisms are resolved in other functions is by having the interface (the callable global function `chebyshev_T()`) dispatching on the keyword.
+While other orthogonal polynomial functions (as well as almost all other symbolic functions) are simply `BuiltinFunction`s or `GinacFunction`s this ticket suffers from the fact that the Cheby functions in Sage allow the `algorithm` keyword (by using a `__call__` method in the superclass). The way such polymorphisms are resolved in other functions is by having the interface (the callable global function `chebyshev_T()`) dispatching on the keyword. OTOH the call `chebyshev_T.eval_algebraic()` is expected to work. The only way this can be resolved and prepared for any deprecations is by having the `chebyshev` class not being a `Function` but creating such an object.
rwst commented 6 years ago
comment:2

The usage chebyshev_T(...algorithm='pari' silently uses normal evaluation because there is no _eval_pari_ member, so Pari was never supported.

rwst commented 6 years ago

Description changed:

--- 
+++ 
@@ -1 +1,3 @@
-While other orthogonal polynomial functions (as well as almost all other symbolic functions) are simply `BuiltinFunction`s or `GinacFunction`s this ticket suffers from the fact that the Cheby functions in Sage allow the `algorithm` keyword (by using a `__call__` method in the superclass). The way such polymorphisms are resolved in other functions is by having the interface (the callable global function `chebyshev_T()`) dispatching on the keyword. OTOH the call `chebyshev_T.eval_algebraic()` is expected to work. The only way this can be resolved and prepared for any deprecations is by having the `chebyshev` class not being a `Function` but creating such an object.
+While other orthogonal polynomial functions (as well as almost all other symbolic functions) are simply `BuiltinFunction`s or `GinacFunction`s this ticket suffers from the fact that the Cheby functions in Sage allow the `algorithm` keyword (by using a `__call__` method in the superclass). The way such polymorphisms are resolved in other functions is by having the interface (the callable global function `chebyshev_T()`) dispatching on the keyword, especially because doctests involving all kind of classes as function argument are expected to work eternally.
+
+The goal of this ticket is to move the `__call__` method out of the `Function` class (as well as other methods used as interface) into a separate interface class; to 2. make the remaining class inherit from `BuiltinFunction` and, 3., to remove the `ChebyshevFunction` superclass as it is then no longer used.
rwst commented 6 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,3 @@
-While other orthogonal polynomial functions (as well as almost all other symbolic functions) are simply `BuiltinFunction`s or `GinacFunction`s this ticket suffers from the fact that the Cheby functions in Sage allow the `algorithm` keyword (by using a `__call__` method in the superclass). The way such polymorphisms are resolved in other functions is by having the interface (the callable global function `chebyshev_T()`) dispatching on the keyword, especially because doctests involving all kind of classes as function argument are expected to work eternally.
+While other orthogonal polynomial functions (as well as almost all other symbolic functions) are simply `BuiltinFunction`s or `GinacFunction`s this ticket suffers from the fact that the Cheby functions in Sage allow the `algorithm` keyword (by using a `__call__` method in the superclass). The way such polymorphisms are resolved in other functions is by having the interface (the callable global function `chebyshev_T()`) dispatching on the keyword, especially because doctests involving all kind of Sage objects as function argument are expected to work eternally.

 The goal of this ticket is to move the `__call__` method out of the `Function` class (as well as other methods used as interface) into a separate interface class; to 2. make the remaining class inherit from `BuiltinFunction` and, 3., to remove the `ChebyshevFunction` superclass as it is then no longer used.
rwst commented 6 years ago
comment:5

Also the usage chebyshev_T(...algorithm='maxima') used normal evaluation. Maxima returns a polynomial in (x-1) which probably is useful so I'm changing doctests. In a later step this could be renamed to a more descriptive name and computed by Sage.

rwst commented 6 years ago
comment:6

Also eval_formula() should become algorithm='hypergeometric'. The Maxima "algorithm" is variant of it, see https://en.wikipedia.org/wiki/Chebyshev_polynomials#Explicit_expressions

rwst commented 6 years ago

Branch: u/rws/remove_call___usage_in_chebyshev_functions

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 6 years ago

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

23190a624554: refactor chebyshev_U
258233024554: remove ChebyshevFunction
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 6 years ago

Commit: 2582330

rwst commented 6 years ago

Author: Ralf Stephan

rwst commented 6 years ago
comment:10

Docs fail to build, doctest errors in three files:

rwst commented 6 years ago

Changed branch from u/rws/remove_call___usage_in_chebyshev_functions to u/rws/24554

rwst commented 6 years ago

New commits:

957ce1424554: Refactoring in Chebyshev functions
rwst commented 6 years ago

Changed commit from 2582330 to 957ce14

videlec commented 4 years ago
comment:13

merge failure

mkoeppe commented 4 years ago
comment:15

Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.

mkoeppe commented 3 years ago
comment:17

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

mkoeppe commented 3 years ago
comment:18

Setting a new milestone for this ticket based on a cursory review.