Opened 7 years ago
Closed 5 years ago
#13246 closed defect (fixed)
Automatic exclusion of nondomain points in things like arcsec
Reported by:  kcrisman  Owned by:  jason, was 

Priority:  major  Milestone:  sage6.3 
Component:  graphics  Keywords:  
Cc:  ddrake, ppurka, jondo, vbraun  Merged in:  
Authors:  Punarbasu Purkayastha  Reviewers:  Ralf Stephan, KarlDieter Crisman 
Report Upstream:  N/A  Work issues:  
Branch:  384b5a2 (Commits)  Commit:  384b5a23011b43380be2c434e2487121aaa7dea8 
Dependencies:  #16439  Stopgaps: 
Description (last modified by )
From an email exchange about plotting arcsec, slightly out of order:
The immediate context for this issue is a couple of problems that include, among other things, graphing arcsec and its derivative (in one case), arcsec and arccsc (in the other). Since the range of intended functions is known, it appears that this would work if the students follow instructions (big if). But more generally, we want students to have total control over their "graphing calculator"  at least over the input boxes we provide  so there's nothing stopping them from entering arcsec(x/2) or something else that we can't anticipate.
The underlying problem is that the plotting code ignores everything between 1 and 1 (since those values aren't real), but then simply connects (1,3) and (1,.2) (or so) with a line segment when it's done. I have a couple ideas for fixing this, but they won't help you in the near term.
One workaround for your @interact would be to select an option for exclude based on the function: I'm thinking of something like this:
@interact def foo(fn=textbox(), ....):
setup stuff...
if fn == 'arcsin':
exclude=[]
elif fn == 'arcsec':
exclude=[1,1]
plot(fn, ...., exclude=exclude)
The idea is just to look at the function being plotted and set a list of points to exclude based on that. It would be tedious to code, but would work.
I think that fixing this is possible. I spent about a half hour looking at http://hg.sagemath.org/sagemain/file/9ab4ab6e12d0/sage/plot/plot.py just now and I am pretty sure that one could extract bad points like nan in generate_plot_points. Currently we just do
data = [data[i] for i in range(len(data)) if i not in exception_indices]
which means we completely ignore them, but presumably we could keep this data somehow and return it as part of generate_plot_points (not sure whether that would have to be deprecated) so that we could add that somehow to the exclude data around http://hg.sagemath.org/sagemain/file/9ab4ab6e12d0/sage/plot/plot.py#l1279
The problem is that there could be a lot of these, and that could slow things down a lot to check all of them, but checking for a "consecutive series" would also take some time.
Anyway, it's at least doable in principle, but would take some time and care to implement, making sure it didn't cause any other regressions.
Apply to devel/sage:
Attachments (5)
Change History (75)
Changed 7 years ago by
comment:1 Changed 7 years ago by
comment:2 followup: ↓ 3 Changed 7 years ago by
 Cc ppurka added
Hey ppurka, I know you like a challenge and know the plotting code pretty well... :)
comment:3 in reply to: ↑ 2 Changed 7 years ago by
Replying to kcrisman:
Hey ppurka, I know you like a challenge and know the plotting code pretty well... :)
Hehe, I will have a look. :)
This problem had come up in sagesupport or sagedevel too a couple of months back.
comment:4 Changed 7 years ago by
Here's an idea I thought of that would help avoid these ugly situations. The problem is that we build our plot out of lots of tiny line segments, and we have a line segment connecting things that it should not connect. So:
Let w = xmax  xmin
. After building the list of (x,y)
points for the plot, go through the x
s. If the difference between any two consecutive x
values is "too big", don't connect them.
Since going through the list would be expensive and rarely needed, we would only do this with a keyword, say detect_gaps
. If set to True
, it would default to ignoring gaps of, say, 0.01*w
. Otherwise, the user could specify a proportion: detect_gaps=0.05
or whatever. This procedure would only run if detect_gaps
was True
or a number between 0 and 1.
Does that seem reasonable? That would fix the original problem at the cost of a little speed.
comment:5 Changed 7 years ago by
@ddrake: I guess the point of this ticket is to make the exclusion automatic. Surely, there will be a slowdown if we do this automatically for every plot; the question is what will be the magnitude of the slowdown. I suspect there won't be much slowdown with the default plot_points=200
.
Your solution also sounds plausible. In case we do adopt your solution, I would like it to be a part of the exclude
keyword in the plot command since that is already being used to exclude user defined points from the plot. Something like exclude='automatic'
for a default proportion of 0.05 or exclude=0.02
for a user defined proportion. Hopefully, this won't break the existing code and it also won't introduce any new keywords. There are already two such keywords which deal with not plotting some points  exclude
and detect_poles
.
comment:6 Changed 7 years ago by
Hmm, this idea is not bad. Particularly since we pick a default step size for the x
values, one could simply choose some multiple of that (like 1 or 2) as the minimum to remove in that case. There are some issues with making sure behavior is still right for generate_plot_points
in nonnormal cases (no recursion, etc.) but this has some promise.
comment:7 Changed 7 years ago by
Can you check many different plots with and without this patch? It should fix two things:
 automatically exclude invalid regions
 fix a bug in the exclude code. This bug can be found if you try to plot the following for instance and expect the point 1.5 to be excluded.
plot(arcsec, 2, 2, exclude=[1.5, 0, 0.5, 1.5])
comment:8 Changed 7 years ago by
Hold on.. this patch is not right. It has broken at least two plots:
plot(sin(1/x), (x, 1, 1)) plot(sin(x), (x, 0, 10), linestyle='.')
The first one I can understand, but I don't understand the problem with the second one.
Edit: Can't reproduce this. See comment:10.
comment:9 Changed 7 years ago by
My totally gut guess is that you're not taking adaptive recursion into account enough. Of course, one would think that would make the distance between plot points even smaller, but who knows?
I'd insert a print statement in your while loop to see what's being excluded. Also, does sin
break only with the linestyle, or in general?
comment:10 Changed 7 years ago by
Actually, I am trying to reproduce it on my laptop now, and I can't reproduce the failures. I don't know what happened back then. I was manually going through each plot command in the docstrings and plotting it and verifying visually whether it was all right. For both the above examples I was getting a straight horizontal line. Now, when I am plotting it again, I don't see any anomalies. :/
I will let this patch bake for a couple more days, before I set it up for needs_review
. Maybe it corrects itself automatically over time.
comment:11 Changed 7 years ago by
 Status changed from new to needs_review
I manually and visually (!) tested all the plots in the doctests of sage/plot/plot.py
and this patch hasn't adversely affected any of them. I also changed the test to 2 * (distance between two xaxis points)
.
Setting this up for review. If you know a bunch of illbehaving functions like arcsec
then please mention them here so that I can run more tests.
comment:12 Changed 7 years ago by
 Description modified (diff)
comment:13 Changed 7 years ago by
Please fill in your real name as Author.
comment:14 Changed 7 years ago by
comment:15 Changed 7 years ago by
 Status changed from needs_review to needs_work
 Work issues set to fix doctests
comment:16 Changed 7 years ago by
 Status changed from needs_work to needs_review
 Work issues fix doctests deleted
comment:17 Changed 6 years ago by
Notice this (unsurprisingly) needs a slight rebase.
Impressively, this also seems to work correctly with things like
sage: parametric_plot( (arcsec(x), x^2), (x,2,2))
although
parametric_plot( (x,arcsec(x)), (x,2,2))
would cause an error always  something that probably should be fixed, perhaps this is a good place to do so? Note that there is no helpful error message like with
WARNING: When plotting, failed to evaluate function at 99 points.
I'm trying to rack my brain thinking of where 2 times the difference between two consecutive plot points
might cause a problem in the parametric case. Even
parametric_plot( (100000*arcsec(x), x), (x,2,2),aspect_ratio='automatic')
doesn't seem to trigger additional exclusions. Do you see what I mean? One could imagine that in a parametric context there could be a very narrow parameter range, but a lot of room on the xvalues and so everything would be excluded.
Changed 6 years ago by
comment:18 Changed 6 years ago by
Apply trac_13246rebase.patch
comment:19 Changed 6 years ago by
 Description modified (diff)
Thanks for rebasing. I have attached a new patch to take care of the failing parametric plot too.
Apply trac_13246rebase.patch trac_13246parametric.patch
comment:20 Changed 6 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:21 Changed 6 years ago by
 Cc jondo added
comment:22 Changed 6 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:23 followup: ↓ 24 Changed 6 years ago by
 Status changed from needs_review to needs_work
Apart from that the patch no longer appliesand I'm not sure if the changes meanwhile done in Sage possibly change the overall situation or the necessary patch, the given doctest fails with AttributeError: 'complex' object has no attribute 'arccos'
.
comment:24 in reply to: ↑ 23 Changed 6 years ago by
Replying to rws:
Apart from that the patch no longer appliesand I'm not sure if the changes meanwhile done in Sage possibly change the overall situation or the necessary patch, the given doctest fails with
AttributeError: 'complex' object has no attribute 'arccos'
.
I haven't considered rebasing this patch (and a couple other patches) because there doesn't seem to be much interest in this. The issue that this patch fixes did come up in the mailing list two or three months ago, but I haven't obtained any feedback since then. As far as I know, this issue is definitely not fixed even in the latest development versions.
The problem with AttributeError
is not introduced with this patch. It seems to be a problem with Sage itself. The code where it fails is
.../sage/local/lib/python2.7/sitepackages/sage/functions/trig.pyc in _evalf_(self, x, parent, algorithm) 732 if parent is float: 733 return math.acos(1/x) > 734 return (1/x).arccos() 735 736 def _eval_numpy_(self, x):
EDIT: Sorry, I was wrong. The use of instance(parent, (float, complex))
is not correct since complex
can not be converted to float
apparently. In that case, there is something else wrong in Sage that is producing complex
entries, when the entries were float
earlier.
comment:25 Changed 6 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:26 Changed 6 years ago by
Here's a recent email, though:
Guys: I'm not sure how to comment on ticket #13246, but it appears that the problem of correctly plotting arcsec x and other functions with large domain gaps is still not solved. Any chance for progress? Thanks.
comment:27 Changed 6 years ago by
The error about complex entries was introduced in between 5.10.rc1 and 5.11.rc1
Earlier it used to be a ValueError
and was properly handled by the plot code:
sage: from sage.plot.misc import setup_for_eval_on_grid sage: f, d = setup_for_eval_on_grid(arcsec, [(2, 2)]) sage: xi = 0.992725195401 sage: f(xi)  ValueError Traceback (most recent call last) <ipythoninput4f7215b663c19> in <module>() > 1 f(xi) /usr/local/src/sage/sage5.10.rc1.server/local/lib/python2.7/sitepackages/sage/ext/interpreters/wrapper_rdf.so in sage.ext.interpreters.wrapper_rdf.Wrapper_rdf.__call__ (build/cythonized/sage/ext/interpreters/wrapper_rdf.c:1641)() /usr/local/src/sage/sage5.10.rc1.server/local/lib/python2.7/sitepackages/sage/symbolic/expression.so in sage.symbolic.expression.Expression.__float__ (build/cythonized/sage/symbolic/expression.cpp:7218)() /usr/local/src/sage/sage5.10.rc1.server/local/lib/python2.7/sitepackages/sage/symbolic/expression.so in sage.symbolic.expression.Expression._eval_self (build/cythonized/sage/symbolic/expression.cpp:6246)() /usr/local/src/sage/sage5.10.rc1.server/local/lib/python2.7/sitepackages/sage/functions/trig.pyc in _evalf_(self, x, parent) 731 """ 732 if parent is float: > 733 return math.acos(1/x) 734 return (1/x).arccos() 735 ValueError: math domain error
Now it spits out AttributeError
instead.
sage: from sage.plot.misc import setup_for_eval_on_grid sage: f, d = setup_for_eval_on_grid(arcsec, [(2, 2)]) sage: xi = 0.992725195401 sage: f(xi)  AttributeError Traceback (most recent call last) <ipythoninput4f7215b663c19> in <module>() > 1 f(xi) /usr/local/src/sage/sage5.11.rc0/local/lib/python2.7/sitepackages/sage/ext/interpreters/wrapper_rdf.so in sage.ext.interpreters.wrapper_rdf.Wrapper_rdf.__call__ (sage/ext/interpreters/wrapper_rdf.c:1641)() /usr/local/src/sage/sage5.11.rc0/local/lib/python2.7/sitepackages/sage/symbolic/function.so in sage.symbolic.function.BuiltinFunction.__call__ (sage/symbolic/function.cpp:8029)() /usr/local/src/sage/sage5.11.rc0/local/lib/python2.7/sitepackages/sage/symbolic/expression.so in sage.symbolic.expression.Expression.__complex__ (sage/symbolic/expression.cpp:7805)() /usr/local/src/sage/sage5.11.rc0/local/lib/python2.7/sitepackages/sage/symbolic/expression.so in sage.symbolic.expression.Expression._eval_self (sage/symbolic/expression.cpp:6696)() /usr/local/src/sage/sage5.11.rc0/local/lib/python2.7/sitepackages/sage/functions/trig.pyc in _evalf_(self, x, parent) 732 if parent is float: 733 return math.acos(1/x) > 734 return (1/x).arccos() 735 736 def _eval_numpy_(self, x): AttributeError: 'complex' object has no attribute 'arccos'
comment:28 Changed 6 years ago by
 Cc vbraun added
What do you make of this? Bug in cython? cc: vbraun
sage: from sage.plot.misc import setup_for_eval_on_grid sage: f, d = setup_for_eval_on_grid(arcsec, [(2, 2)]) sage: f(0.992725195401)  AttributeError Traceback (most recent call last) <ipythoninput3a06f3eb7afeb> in <module>() > 1 f(RealNumber('0.992725195401')) /home/basu/Installations/sage/local/lib/python2.7/sitepackages/sage/ext/interpreters/wrapper_rdf.so in sage.ext.interpreters.wrapper_rdf.Wrapper_rdf.__call__ (sage/ext/interpreters/wrapper_rdf.c:1689)() /home/basu/Installations/sage/local/lib/python2.7/sitepackages/sage/symbolic/function.so in sage.symbolic.function.BuiltinFunction.__call__ (sage/symbolic/function.cpp:9088)() /home/basu/Installations/sage/local/lib/python2.7/sitepackages/sage/symbolic/expression.so in sage.symbolic.expression.Expression.__complex__ (sage/symbolic/expression.cpp:8174)() /home/basu/Installations/sage/local/lib/python2.7/sitepackages/sage/symbolic/expression.so in sage.symbolic.expression.Expression._eval_self (sage/symbolic/expression.cpp:6973)() /home/basu/Installations/sage/local/lib/python2.7/sitepackages/sage/functions/trig.pyc in _evalf_(self, x, parent, algorithm) 732 if parent is float: 733 return math.acos(1/x) > 734 return (1/x).arccos() 735 736 def _eval_numpy_(self, x): AttributeError: 'complex' object has no attribute 'arccos' sage: Exiting Sage (CPU time 0m0.25s, Wall time 0m12.78s). basu@plantain:~/Installations/sage [testfloat] $ git diff develop cat # insert a print statement diff git a/src/sage/symbolic/function.pyx b/src/sage/symbolic/function.pyx index eed56e6..75c723d 100644  a/src/sage/symbolic/function.pyx +++ b/src/sage/symbolic/function.pyx @@ 934,6 +934,7 @@ cdef class BuiltinFunction(Function): # conversion to the original parent failed # we try if it works with the corresponding complex domain if org_parent is float: + print "DEBUG: it is float  {}".format(float(res)) try: return complex(res) except (TypeError, ValueError): basu@plantain:~/Installations/sage [testfloat] $ ./sage b >& /dev/null ; ./sage ┌────────────────────────────────────────────────────────────────────┐ │ Sage Version 6.3.beta2, Release Date: 20140524 │ │ Type "notebook()" for the browserbased notebook interface. │ │ Type "help()" for help. │ └────────────────────────────────────────────────────────────────────┘ ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Warning: this is a prerelease version, and it may be unstable. ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ sage: from sage.plot.misc import setup_for_eval_on_grid sage: f, d = setup_for_eval_on_grid(arcsec, [(2, 2)]) sage: f(0.992725195401)  ValueError Traceback (most recent call last) <ipythoninput3a06f3eb7afeb> in <module>() > 1 f(RealNumber('0.992725195401')) /home/basu/Installations/sage/local/lib/python2.7/sitepackages/sage/ext/interpreters/wrapper_rdf.so in sage.ext.interpreters.wrapper_rdf.Wrapper_rdf.__call__ (sage/ext/interpreters/wrapper_rdf.c:1689)() /home/basu/Installations/sage/local/lib/python2.7/sitepackages/sage/symbolic/function.so in sage.symbolic.function.BuiltinFunction.__call__ (sage/symbolic/function.cpp:9099)() /home/basu/Installations/sage/local/lib/python2.7/sitepackages/sage/symbolic/expression.so in sage.symbolic.expression.Expression.__float__ (sage/symbolic/expression.cpp:8022)() /home/basu/Installations/sage/local/lib/python2.7/sitepackages/sage/symbolic/expression.so in sage.symbolic.expression.Expression._eval_self (sage/symbolic/expression.cpp:6973)() /home/basu/Installations/sage/local/lib/python2.7/sitepackages/sage/functions/trig.pyc in _evalf_(self, x, parent, algorithm) 731 """ 732 if parent is float: > 733 return math.acos(1/x) 734 return (1/x).arccos() 735 ValueError: math domain error sage:
The error changes to ValueError
from AttributeError
. How can a print statement change the call sequence?
comment:29 Changed 6 years ago by
 Branch set to u/ppurka/automatic_exclusion_of_non_domain_points_in_things_like_arcsec
comment:30 Changed 6 years ago by
 Commit set to c31f63ed3860dbf2368ed17b3f54d8dd0ebf0331
I pushed the rebased branch (only whitespace errors apparently). The error mentioned about complex numbers is not introduced by the plot code.
New commits:
4193ca4  This patch fixes two things, both related to exclusion of invalid plot points.

c31f63e  fix parametric plots that fail due to invalid plot points

comment:31 Changed 6 years ago by
Thanks for this digging, ppurka. I have to say there isn't a lot in that changelog that seems likely. Maybe #13933? But see below.
This ticket has exposed some bugs in trig.py as well.
sage: arcsec(0.992725195401) NaN sage: arcsec(float(0.992725195401)) <snip> 732 if parent is float: 733 return math.acos(1/x) > 734 return (1/x).arccos() 735 736 def _eval_numpy_(self, x): AttributeError: 'complex' object has no attribute 'arccos'
Compare to behavior in Sage 5.2:
sage: arcsec(0.992725195401) arcsec(0.992725195401000) sage: arcsec(float(0.992725195401)) arcsec(0.992725195401) sage:
Related, I think  none of these take things nicely.
sage: arccos(1+2*i) arccos(2*I + 1) sage: arccos(CC(1,2)) 1.14371774040242  1.52857091948100*I sage: arccos(CDF(1,2)) 1.1437177404  1.52857091948*I sage: arccos(complex(1,2)) <snip> TypeError: Unable to convert x (='(1+2j)') to real number.
Anyway, I think I got it. Old behavior:
sage: arcsec(float(0.992725195401)) arcsec(0.992725195401)
This is because before #13933, arcsec.__call__
, a BuiltinFunction
(not GinacFunction
) was inheriting its __call__
method directly from Function
. Then we hit this branch of the code which turns the argument into a symbolic argument.
But now, arcsec
(still a BuiltinFunction
now) uses the __call__
method from what used to be the method for only GinacFunction
, but which is now the method also for BuiltinFunction
. So after doing the same thing it did before, getting the symbolic version
sage: a = float(0.992725195401) KeyboardInterrupt sage: res = super(sage.symbolic.function.BuiltinFunction,arcsec).__call__(a,coerce=True) sage: res arcsec(0.992725195401)
it now continues on instead of returning, and tries to evaluate res
as a complex, which causes us to go back to _evalf_
of arcsec
sage: complex(res)  AttributeError: 'complex' object has no attribute 'arccos'
I'm not sure if the correct fix is to just make sure trig.py can handle this, or whether this is a possible bug in the way BuiltinFunction
was changed. I'm going to ask at #13933.
Finally, from one of the original requesters, some more test cases.
Thanks for looking into this. The following page is a problem set in our online text that has six interacts, the last two with my workaround for handling arcsec(x) and arccsc(x), as well as their derivatives, which involve 1/sqrt(x^{21), another good test function, along with its reciprocal, sqrt(x}21).
http://calculuscourse.maa.org/main2/Chapter5/Section55/Chapter556M.html
If you enter any of these in those last two "Graph" interacts (Problem 6 or 7), they will be drawn correctly  because nothing is drawn between 1 and 1. [But arcsec(x/2) will cause an error message, and arcsec(2*x) will not be completely drawn.] If you enter any of these in the third or fourth interacts (Problems 4 and 5), there will either be an error message (arcsec or arccsc) or the graph will be incorrectly drawn between 1 and 1 (sqrt(x^{21) or 1/sqrt(x}21)).
I think the general class of functions for which this particular problem exists is those for which the domain is interrupted by an interval of finite length greater than 0. The inverse trig functions seem to involve a different problem when trying to compute nonreal values, but I don't understand the error messages.
comment:32 Changed 6 years ago by
I've opened #16439 for the trig stuff in general. I'm not sure yet whether I want to open a new ticket for the current error, though I'm inclined to so that this can get in. (Though I haven't reviewed it yet!)
comment:33 followup: ↓ 34 Changed 6 years ago by
Also, does this change completely get rid of the whole warnings for soandso many unplotted points? I am wondering whether we might want to include that still anyway for instances where exclude
wasn't set by the user  because sometimes people won't realize it. For instance, now presumably plotting log(x)
from 1 to 1 won't raise a warning any more, but that would be a regression.
comment:34 in reply to: ↑ 33 ; followup: ↓ 35 Changed 6 years ago by
Replying to kcrisman:
Also, does this change completely get rid of the whole warnings for soandso many unplotted points? I am wondering whether we might want to include that still anyway for instances where
exclude
wasn't set by the user  because sometimes people won't realize it. For instance, now presumably plottinglog(x)
from 1 to 1 won't raise a warning any more, but that would be a regression.
Where do you see this behavior? I can not reproduce it. This is the output in sage6.1.1
basu@plantain:~/Installations/sage [t/13246/automat] $ ~/Installations/sage6.1.1.server/sage ┌────────────────────────────────────────────────────────────────────┐ │ Sage Version 6.1.1, Release Date: 20140204 │ │ Type "notebook()" for the browserbased notebook interface. │ │ Type "help()" for help. │ └────────────────────────────────────────────────────────────────────┘ sage: plot(log, 1, 1) verbose 0 (2411: plot.py, generate_plot_points) WARNING: When plotting, failed to evaluate function at 100 points. verbose 0 (2411: plot.py, generate_plot_points) Last error message: 'can't convert complex to float'
And this is the almost same (only change in line numbers) output after this patch
basu@plantain:~/Installations/sage [t/13246/automat] $ ./sage ┌────────────────────────────────────────────────────────────────────┐ │ Sage Version 6.3.beta2, Release Date: 20140524 │ │ Type "notebook()" for the browserbased notebook interface. │ │ Type "help()" for help. │ └────────────────────────────────────────────────────────────────────┘ ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Warning: this is a prerelease version, and it may be unstable. ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ sage: plot(log(x), 1, 1) verbose 0 (2467: plot.py, generate_plot_points) WARNING: When plotting, failed to evaluate function at 100 points. verbose 0 (2467: plot.py, generate_plot_points) Last error message: 'can't convert complex to float'
comment:35 in reply to: ↑ 34 Changed 6 years ago by
Where do you see this behavior? I can not reproduce it. This is the output in sage6.1.1
Oh, great! (Sorry, I hadn't tried this branch yet because of not wanting to deal with some rebuild issues because the branch I'm on has a couple (s)pkg changes.)
But I suspect that anyway we don't have that warning any more unless it's at the endpoints of the domain that we have the problem, right? So I guess I'm just wondering whether, to provide the same service we did before, still printing a warning when there was exclusion of nondomain points (and providing an explicit example in the documentation of how to set the verbosity to not print the warning) would be useful. I'm inclined to say yes.
comment:36 Changed 6 years ago by
Ok. I can't test this now with arcsec
all broken and all. But my doctest has a set_verbose(1)
before plotting the arcsec
. I suspect that was done to suppress exactly the kind of information that you want to display  this was probably done to make the doctest cleaner.
comment:37 Changed 6 years ago by
Fair enough.
Regarding arcsec
 do we want to then have this rely on #16439, or should we just try to get this in anyway, perhaps with some of the other doctests suggested at the end of comment:31?
comment:38 Changed 6 years ago by
 Dependencies set to #16439
In my opinion, we should wait on #16439 so that the doctests pass successfully.
comment:39 Changed 6 years ago by
 Milestone changed from sage6.3 to sagepending
We could make this sagepending in that case.
comment:40 Changed 6 years ago by
 Commit changed from c31f63ed3860dbf2368ed17b3f54d8dd0ebf0331 to 148aafbc155f5fceb9f156a725067e1b9f5c5436
Branch pushed to git repo; I updated commit sha1. New commits:
a18bf04  Merge branch 'develop' into t/13246/automatic_exclusion_of_non_domain_points_in_things_like_arcsec

b34ee39  allow some trigonometric functions to take complex as input

c4ba30d  add couple of doctests for complex input

dd2ac84  Merge branch 't/16439/allow_inverse_trig_functions_to_all_take_complex_input' into t/13246/automatic_exclusion_of_non_domain_points_in_things_like_arcsec

148aafb  add more doctests for auto exclusion functionality

comment:41 Changed 6 years ago by
 Milestone changed from sagepending to sage6.3
 Status changed from needs_work to needs_review
comment:42 followup: ↓ 43 Changed 5 years ago by
 Reviewers set to Ralf Stephan
While the plot doctests pass, in interactive Sage I get
sage: plot(sqrt(x^21), 2, 2) verbose 0 (2474: plot.py, generate_plot_points) WARNING: When plotting, failed to evaluate function at 100 points. verbose 0 (2474: plot.py, generate_plot_points) Last error message: 'math domain error'
before the plot.
In the plot docs, "This reduces the possibility of, e.g., sampling sin only" please render sin via latex, or backticks.
comment:43 in reply to: ↑ 42 Changed 5 years ago by
While the plot doctests pass, in interactive Sage I get
sage: plot(sqrt(x^21), 2, 2) verbose 0 (2474: plot.py, generate_plot_points) WARNING: When plotting, failed to evaluate function at 100 points. verbose 0 (2474: plot.py, generate_plot_points) Last error message: 'math domain error'before the plot.
As discussed above, that is actually desired behavior.
Changed 5 years ago by
comment:44 followups: ↓ 46 ↓ 52 Changed 5 years ago by
parametric_plot((x, arcsec(x)), (x, 2, 2)) <usual complex to float error>
from the doctest doesn't seem to work for me. I wonder if this worked before #16439?
I have a different comment.
sage: plot(sqrt(x^21), 2, 2, plot_points=500, ymin=0)
yields As you can see, it never quite makes it down to the horizontal axis. If you use the endpoints 1 and 2, it does; 0 and 2 doesn't. (And presumably didn't in the past, either.) Anyway, that is annoying, though I'm not sure there is much to do about it.
comment:45 Changed 5 years ago by
 Reviewers changed from Ralf Stephan to Ralf Stephan, KarlDieter Crisman
comment:46 in reply to: ↑ 44 ; followup: ↓ 49 Changed 5 years ago by
Replying to kcrisman:
parametric_plot((x, arcsec(x)), (x, 2, 2)) <usual complex to float error>from the doctest doesn't seem to work for me. I wonder if this worked before #16439?
It does work when you checkout this branch only. I didn't report it because I thought the version of the file in this ticket was more uptodate.
comment:47 Changed 5 years ago by
 Commit changed from 148aafbc155f5fceb9f156a725067e1b9f5c5436 to a01095ea161f0adae68692c67174771232c80675
Branch pushed to git repo; I updated commit sha1. New commits:
a01095e  fix latex according to reviewer comments

comment:48 Changed 5 years ago by
@kcrisman: The parametric plot works here. See http://i.imgur.com/YejcPZq.jpg
@rws: the doctests don't show the warnings because I set verbose(1)
before the plot.
comment:49 in reply to: ↑ 46 ; followup: ↓ 50 Changed 5 years ago by
from the doctest doesn't seem to work for me. I wonder if this worked before #16439?
It does work when you checkout this branch only. I didn't report it because I thought the version of the file in this ticket was more uptodate.
That's odd, and I can confirm what you say. I clearly don't get the wonders of git yet. Sigh. (But what is different here from there with respect to complex/float?)
comment:50 in reply to: ↑ 49 ; followup: ↓ 51 Changed 5 years ago by
Replying to kcrisman:
I clearly don't get the wonders of git yet. Sigh.
May I suggest my write up ;)
comment:51 in reply to: ↑ 50 Changed 5 years ago by
I clearly don't get the wonders of git yet. Sigh.
May I suggest my write up
;)
Maybe you need to find a way to include some of this in the developer guide...
comment:52 in reply to: ↑ 44 Changed 5 years ago by
As you can see, it never quite makes it down to the horizontal axis. If you use the endpoints 1 and 2, it does; 0 and 2 doesn't. (And presumably didn't in the past, either.) Anyway, that is annoying, though I'm not sure there is much to do about it.
Here is what is going on. For userprovided exclusion, we have
if isinstance(exclude, (list, tuple)): exclude = sorted(exclude) # We make sure that points plot points close to the excluded points are computed
but we don't have something similar to that for the 'autoexcluded points'. I don't see an easy way to do this, though.
I am also annoyed (this has nothing to do with this patch) by the inconsistency between exclude is not None
, not exclude
, exclude != None
, and so forth. There are subtle things that can happen here, though I think that in this case they don't.
Now for something that I still see as a bug, though probably not a problem for this ticket.
sage: polar_plot(sin(sqrt(x^21)), (x, 0, 2*pi), exclude=[2,3]) sage: polar_plot(sin(sqrt(x^21)), (x, 0, 2*pi), exclude=[1/2,2,3])
Compare these. The first one does just what you want. The second one does not  presumably because I'm excluding a point already not in the domain, but why does that cause a problem? The others should still be there. You could say it's user error, but then again
sage: plot(x,(x,0,1),exclude=[1,1/2])
works as desired, so I don't think that is why, and it occurs with and without this change. Actually, presumably this is via parametric from polar.
sage: parametric_plot((sqrt(x^21),sqrt(x^21/2)),(x,0,5), exclude=[2,3]) sage: parametric_plot((sqrt(x^21),sqrt(x^21/2)),(x,0,5), exclude=[1,2,3])
Is this a new ticket, or a wontfix? I'm inclined to say this is a bug.
comment:53 Changed 5 years ago by
Oh darn. Touching this exclude this has opened up a full set of bugs. I have fixed the exclude is not != None
kind of stuff but the parametric plot stuff is a different beast.
The problems with parametric plot appear mainly because of a disconnect between the actual data points on the graph and the exclude points  the exclude points are not the actual data points (that appear on the plot) but are actually the values of the parametric variable.
comment:54 Changed 5 years ago by
 Commit changed from a01095ea161f0adae68692c67174771232c80675 to 5c7fb75e184b2e034d53acf1807c58e2165ac7d0
Branch pushed to git repo; I updated commit sha1. New commits:
5c7fb75  do not use exclude both as list and as None

comment:55 Changed 5 years ago by
I am not clear how to fix the parametric plot. I can look into it in detail only over the weekend.
comment:56 Changed 5 years ago by
 Commit changed from 5c7fb75e184b2e034d53acf1807c58e2165ac7d0 to b90cb76eb10225c28257081feafdd4bb3effbd40
Branch pushed to git repo; I updated commit sha1. New commits:
b90cb76  sort and reverse the excluded_points in one command

comment:57 Changed 5 years ago by
 Commit changed from b90cb76eb10225c28257081feafdd4bb3effbd40 to e79bdb8e73d8ebdca5cb6782ca1e4ae7cd17b96e
Branch pushed to git repo; I updated commit sha1. New commits:
e79bdb8  fix parametric and polar plots if exclude point is outside the domain

comment:58 Changed 5 years ago by
I think I have fixed the problem with the parametric and polar plots. Needs review. :)
comment:59 followup: ↓ 60 Changed 5 years ago by
 Status changed from needs_review to positive_review
Thank you for all your very hard work on this!
comment:60 in reply to: ↑ 59 Changed 5 years ago by
Replying to kcrisman:
Thank you for all your very hard work on this!
Thanks for your extensive testing! You have uncovered many bugs in this ticket :)
comment:61 Changed 5 years ago by
 Status changed from positive_review to needs_work
I get
sage t long src/sage/plot/plot.py ********************************************************************** File "src/sage/plot/plot.py", line 1276, in sage.plot.plot.? Failed example: parametric_plot((x, arcsec(x)), (x, 2, 2)) Exception raised: Traceback (most recent call last): File "/home/release/Sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 480, in _run self.execute(example, compiled, test.globs) File "/home/release/Sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 839, in execute exec compiled in globs File "<doctest sage.plot.plot.?[14]>", line 1, in <module> parametric_plot((x, arcsec(x)), (x, Integer(2), Integer(2))) File "/home/release/Sage/local/lib/python2.7/sitepackages/sage/misc/decorators.py", line 550, in wrapper return func(*args, **options) File "/home/release/Sage/local/lib/python2.7/sitepackages/sage/plot/plot.py", line 1659, in parametric_plot return plot(funcs, *args, **kwargs) File "/home/release/Sage/local/lib/python2.7/sitepackages/sage/misc/decorators.py", line 705, in wrapper return func(*args, **kwds) File "/home/release/Sage/local/lib/python2.7/sitepackages/sage/misc/decorators.py", line 550, in wrapper return func(*args, **options) File "/home/release/Sage/local/lib/python2.7/sitepackages/sage/plot/plot.py", line 1176, in plot G = _plot(funcs, *args, **kwds) File "/home/release/Sage/local/lib/python2.7/sitepackages/sage/plot/plot.py", line 1383, in _plot newdata.append((fdata, g(x))) File "wrapper_rdf.pyx", line 80, in sage.ext.interpreters.wrapper_rdf.Wrapper_rdf.__call__ (build/cythonized/sage/ext/interpreters/wrapper_rdf.c:1689) TypeError: can't convert complex to float
comment:62 Changed 5 years ago by
comment:63 Changed 5 years ago by
hmm.. git is not merging the two tickets correctly. I will have to check.
comment:64 Changed 5 years ago by
 Commit changed from e79bdb8e73d8ebdca5cb6782ca1e4ae7cd17b96e to 384b5a23011b43380be2c434e2487121aaa7dea8
Branch pushed to git repo; I updated commit sha1. New commits:
384b5a2  catch TypeError after #16439 is applied

comment:65 Changed 5 years ago by
 Status changed from needs_work to needs_review
@vbraun: this should fix the doctest error. This ticket by itself was working fine. Applying this after #16439 introduced a TypeError
that I wasn't catching. Now it all works fine.
Edit: I think it is because of this git commit in #16439 (that is not merged into this ticket) that a TypeError
started getting raised.
comment:66 Changed 5 years ago by
 Status changed from needs_review to needs_work
breaks plot in this case:
sage: var('n,a') sage: hbar, m = 1,1 sage: psi(x,t,n)=sqrt(2/a)*sin(n*pi*x/a)*e^(i*n^2*pi^2*hbar*t/(2*m*a^2)); sage: print psi sage: Psi(x,t)=1/sqrt(2)*psi(x,t,1)+1/sqrt(2)*psi(x,t,2); sage: print Psi sage: P(x,t,a) = Psi.conjugate()*Psi sage: print P.expand() sage: plot(P(x,1,1),x,0,1)
comment:67 Changed 5 years ago by
 Status changed from needs_work to needs_review
False alarm! It is working as intended. :)
p = plot(P(x,1,1),x,0,1) po = p._objects[0] po.xdata [0.0,0.5,1.0]
comment:68 Changed 5 years ago by
I was happy with this, so I guess if Volker can test if the doctest error he saw is gone then we are okay.
comment:69 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:70 Changed 5 years ago by
 Branch changed from u/ppurka/automatic_exclusion_of_non_domain_points_in_things_like_arcsec to 384b5a23011b43380be2c434e2487121aaa7dea8
 Resolution set to fixed
 Status changed from positive_review to closed
arcsec plot