Opened 6 years ago
Closed 6 years ago
#17953 closed defect (fixed)
symbolic function args prevent forced conversion of result to numeric
Reported by: | rws | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.6 |
Component: | symbolics | Keywords: | |
Cc: | jdemeyer | Merged in: | |
Authors: | Ralf Stephan | Reviewers: | Marc Mezzarobba |
Report Upstream: | N/A | Work issues: | |
Branch: | adc230d (Commits, GitHub, GitLab) | Commit: | adc230dff1b357478155978ecc1149d58f282403 |
Dependencies: | Stopgaps: |
Description (last modified by )
It is generally preferred that the returned result type matches the argument type when calling a builtin function. For example this is as expected:
sage: factorial(8).parent() Integer Ring sage: factorial(SR(8)).parent() Symbolic Ring sage: exp(0).parent() Integer Ring sage: exp(SR(0)).parent() Symbolic Ring
but this is not:
from sage.symbolic.function import BuiltinFunction class TestFunction(BuiltinFunction): def __init__(self): BuiltinFunction.__init__(self, "testfun", nargs=2) def _eval_(self, n, x, *args, **kwds): print (parent(n), parent(x)) return SR(5) sage: TestFunction()(SR(1),GF(2)(1)) (Integer Ring, Finite Field of size 2) 5 sage: type(_) <type 'int'>
An explanation could be that factorial
and exp
derive from GinacFunction
.
Change History (13)
comment:1 Changed 6 years ago by
- Description modified (diff)
- Summary changed from inconsistency in returned type of symbolic function result to inconsistency in returned type of BuiltinFunction result
comment:2 Changed 6 years ago by
- Cc jdemeyer added
comment:3 Changed 6 years ago by
- Branch set to u/rws/inconsistency_in_returned_type_of_builtinfunction_result
comment:4 Changed 6 years ago by
- Commit set to f0fe6f4093db35e8ccc1734ef3220ee72162d3e6
- Status changed from new to needs_review
- Summary changed from inconsistency in returned type of BuiltinFunction result to symbolic function args prevent forced conversion of result to numeric
comment:5 follow-up: ↓ 11 Changed 6 years ago by
Hello,
First of all, I think that the content of the branch is ok (I am running the test right now).
But I found the following behavior weird
sage: exp(0).parent() Integer ring
In my opinion, a function should have a well defined domain and range. In some cases, the function preserves some subset. Like the function x -> x^2
defined in the complex numbers preserves integers, rationals and algebraic. But considering that {0}
is a subset of the real numbers on which exp
preserve the integers is going too far.
Vincent
comment:6 Changed 6 years ago by
- Status changed from needs_review to needs_work
sage -t --long src/sage/functions/log.py ********************************************************************** File "src/sage/functions/log.py", line 628, in sage.functions.log.Function_lambert_w._eval_ Failed example: parent(lambert_w(e)) Expected: Integer Ring Got: Symbolic Ring **********************************************************************
comment:7 Changed 6 years ago by
- Commit changed from f0fe6f4093db35e8ccc1734ef3220ee72162d3e6 to adc230dff1b357478155978ecc1149d58f282403
comment:8 Changed 6 years ago by
- Status changed from needs_work to needs_review
comment:9 follow-up: ↓ 10 Changed 6 years ago by
Simpler way of doing thing
symbolic_input = any(parent_c(arg) is SR for arg in args)
comment:10 in reply to: ↑ 9 Changed 6 years ago by
Replying to vdelecroix:
Simpler way of doing
symbolic_input = any(parent_c(arg) is SR for arg in args)
True. Not worth a commit however.
comment:11 in reply to: ↑ 5 Changed 6 years ago by
- Status changed from needs_review to positive_review
Replying to vdelecroix:
In my opinion, a function should have a well defined domain and range. In some cases, the function preserves some subset. Like the function
x -> x^2
defined in the complex numbers preserves integers, rationals and algebraic. But considering that{0}
is a subset of the real numbers on whichexp
preserve the integers is going too far.
I tend to agree, but the present ticket is a step in the right direction in any case: it fixes some of the issues without making it harder to improve things later on.
comment:12 Changed 6 years ago by
- Reviewers set to Marc Mezzarobba
comment:13 Changed 6 years ago by
- Branch changed from u/rws/inconsistency_in_returned_type_of_builtinfunction_result to adc230dff1b357478155978ecc1149d58f282403
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
17953: any symbolic function arg prevents forced result conversion to numeric