Opened 10 years ago

Closed 8 years ago

# Automatic exclusion of non-domain points in things like arcsec

Reported by: Owned by: kcrisman jason, was major sage-6.3 graphics ddrake, ppurka, jondo, vbraun Punarbasu Purkayastha Ralf Stephan, Karl-Dieter Crisman N/A 384b5a2 384b5a23011b43380be2c434e2487121aaa7dea8 #16439

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/sage-main/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/sage-main/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:

arcsec plot

### comment:1 Changed 10 years ago by kcrisman

Here is what the bad plot looks like. This function should not be defined in the middle.

### comment:2 follow-up: ↓ 3 Changed 10 years ago by kcrisman

Hey ppurka, I know you like a challenge and know the plotting code pretty well... :)

### comment:3 in reply to: ↑ 2 Changed 10 years ago by ppurka

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 sage-support or sage-devel too a couple of months back.

### comment:4 Changed 10 years ago by ddrake

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 10 years ago by ppurka

@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`.

Last edited 10 years ago by ppurka (previous) (diff)

### comment:6 Changed 10 years ago by kcrisman

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 non-normal cases (no recursion, etc.) but this has some promise.

### comment:7 Changed 10 years ago by ppurka

Can you check many different plots with and without this patch? It should fix two things:

1. automatically exclude invalid regions
2. 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 10 years ago by ppurka

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.

Last edited 10 years ago by ppurka (previous) (diff)

### comment:9 Changed 10 years ago by kcrisman

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 10 years ago by ppurka

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 10 years ago by ppurka

• 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 x-axis points)`.

Setting this up for review. If you know a bunch of ill-behaving functions like `arcsec` then please mention them here so that I can run more tests.

### comment:12 Changed 10 years ago by ppurka

• Description modified (diff)

### comment:14 Changed 10 years ago by ppurka

• Authors set to Punarbasu Purkayastha

### comment:15 Changed 10 years ago by ppurka

• Status changed from needs_review to needs_work
• Work issues set to fix doctests

### Changed 10 years ago by ppurka

apply to devel/sage

### comment:16 Changed 10 years ago by ppurka

• Status changed from needs_work to needs_review
• Work issues fix doctests deleted

### comment:17 Changed 9 years ago by kcrisman

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 x-values and so everything would be excluded.

### comment:18 Changed 9 years ago by kcrisman

Apply trac_13246-rebase.patch

### Changed 9 years ago by ppurka

apply to devel/sage

### comment:19 Changed 9 years ago by ppurka

• Description modified (diff)

Thanks for rebasing. I have attached a new patch to take care of the failing parametric plot too.

Apply trac_13246-rebase.patch trac_13246-parametric.patch

### comment:20 Changed 9 years ago by jdemeyer

• Milestone changed from sage-5.11 to sage-5.12

### comment:22 Changed 9 years ago by vbraun_spam

• Milestone changed from sage-6.1 to sage-6.2

### comment:23 follow-up: ↓ 24 Changed 8 years ago by rws

• Status changed from needs_review to needs_work

Apart from that the patch no longer applies---and 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 8 years ago by ppurka

Apart from that the patch no longer applies---and 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/site-packages/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.

Last edited 8 years ago by ppurka (previous) (diff)

### comment:25 Changed 8 years ago by vbraun_spam

• Milestone changed from sage-6.2 to sage-6.3

### comment:26 Changed 8 years ago by kcrisman

Here's a recent email, though:

```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?
```
Last edited 15 months ago by slelievre (previous) (diff)

### comment:27 Changed 8 years ago by ppurka

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)
<ipython-input-4-f7215b663c19> in <module>()
----> 1 f(xi)

/usr/local/src/sage/sage-5.10.rc1.server/local/lib/python2.7/site-packages/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/sage-5.10.rc1.server/local/lib/python2.7/site-packages/sage/symbolic/expression.so in sage.symbolic.expression.Expression.__float__ (build/cythonized/sage/symbolic/expression.cpp:7218)()

/usr/local/src/sage/sage-5.10.rc1.server/local/lib/python2.7/site-packages/sage/symbolic/expression.so in sage.symbolic.expression.Expression._eval_self (build/cythonized/sage/symbolic/expression.cpp:6246)()

/usr/local/src/sage/sage-5.10.rc1.server/local/lib/python2.7/site-packages/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)
<ipython-input-4-f7215b663c19> in <module>()
----> 1 f(xi)
/usr/local/src/sage/sage-5.11.rc0/local/lib/python2.7/site-packages/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/sage-5.11.rc0/local/lib/python2.7/site-packages/sage/symbolic/function.so in sage.symbolic.function.BuiltinFunction.__call__ (sage/symbolic/function.cpp:8029)()
/usr/local/src/sage/sage-5.11.rc0/local/lib/python2.7/site-packages/sage/symbolic/expression.so in sage.symbolic.expression.Expression.__complex__ (sage/symbolic/expression.cpp:7805)()
/usr/local/src/sage/sage-5.11.rc0/local/lib/python2.7/site-packages/sage/symbolic/expression.so in sage.symbolic.expression.Expression._eval_self (sage/symbolic/expression.cpp:6696)()
/usr/local/src/sage/sage-5.11.rc0/local/lib/python2.7/site-packages/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 8 years ago by ppurka

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)
<ipython-input-3-a06f3eb7afeb> in <module>()
----> 1 f(-RealNumber('0.992725195401'))

/home/basu/Installations/sage/local/lib/python2.7/site-packages/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/site-packages/sage/symbolic/function.so in sage.symbolic.function.BuiltinFunction.__call__ (sage/symbolic/function.cpp:9088)()

/home/basu/Installations/sage/local/lib/python2.7/site-packages/sage/symbolic/expression.so in sage.symbolic.expression.Expression.__complex__ (sage/symbolic/expression.cpp:8174)()

/home/basu/Installations/sage/local/lib/python2.7/site-packages/sage/symbolic/expression.so in sage.symbolic.expression.Expression._eval_self (sage/symbolic/expression.cpp:6973)()

/home/basu/Installations/sage/local/lib/python2.7/site-packages/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 [test-float] \$ 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 [test-float] \$ ./sage -b >& /dev/null ; ./sage
┌────────────────────────────────────────────────────────────────────┐
│ Sage Version 6.3.beta2, Release Date: 2014-05-24                   │
│ Type "notebook()" for the browser-based 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)
<ipython-input-3-a06f3eb7afeb> in <module>()
----> 1 f(-RealNumber('0.992725195401'))

/home/basu/Installations/sage/local/lib/python2.7/site-packages/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/site-packages/sage/symbolic/function.so in sage.symbolic.function.BuiltinFunction.__call__ (sage/symbolic/function.cpp:9099)()

/home/basu/Installations/sage/local/lib/python2.7/site-packages/sage/symbolic/expression.so in sage.symbolic.expression.Expression.__float__ (sage/symbolic/expression.cpp:8022)()

/home/basu/Installations/sage/local/lib/python2.7/site-packages/sage/symbolic/expression.so in sage.symbolic.expression.Expression._eval_self (sage/symbolic/expression.cpp:6973)()

/home/basu/Installations/sage/local/lib/python2.7/site-packages/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 8 years ago by ppurka

• Branch set to u/ppurka/automatic_exclusion_of_non_domain_points_in_things_like_arcsec

### comment:30 Changed 8 years ago by ppurka

• 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 8 years ago by kcrisman

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(x2-1), another good test function, along with its reciprocal, sqrt(x2-1).

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(x2-1) or 1/sqrt(x2-1)).

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 non-real values, but I don't understand the error messages.

### comment:32 Changed 8 years ago by kcrisman

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 follow-up: ↓ 34 Changed 8 years ago by kcrisman

Also, does this change completely get rid of the whole warnings for so-and-so 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 ; follow-up: ↓ 35 Changed 8 years ago by ppurka

Also, does this change completely get rid of the whole warnings for so-and-so 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.

Where do you see this behavior? I can not reproduce it. This is the output in sage-6.1.1

```basu@plantain:~/Installations/sage [t/13246/automat] \$ ~/Installations/sage-6.1.1.server/sage
┌────────────────────────────────────────────────────────────────────┐
│ Sage Version 6.1.1, Release Date: 2014-02-04                       │
│ Type "notebook()" for the browser-based 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: 2014-05-24                   │
│ Type "notebook()" for the browser-based 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 8 years ago by kcrisman

Where do you see this behavior? I can not reproduce it. This is the output in sage-6.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 non-domain 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 8 years ago by ppurka

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 8 years ago by kcrisman

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 8 years ago by ppurka

• Dependencies set to #16439

In my opinion, we should wait on #16439 so that the doctests pass successfully.

### comment:39 Changed 8 years ago by kcrisman

• Milestone changed from sage-6.3 to sage-pending

We could make this sage-pending in that case.

### comment:40 Changed 8 years ago by git

• 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 8 years ago by ppurka

• Milestone changed from sage-pending to sage-6.3
• Status changed from needs_work to needs_review

Ok. I added some more doctests. We should review #16439 and get that in, and get this in.

There are three functions (in #16439) that still do not work, but probably no one uses them with complex input otherwise it wouldn't have remained buggy for a decade. I suggest we move them to another ticket.

### comment:42 follow-up: ↓ 43 Changed 8 years ago by rws

• Reviewers set to Ralf Stephan

While the plot doctests pass, in interactive Sage I get

```sage: plot(sqrt(x^2-1), -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 8 years ago by kcrisman

While the plot doctests pass, in interactive Sage I get

```sage: plot(sqrt(x^2-1), -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.

### comment:44 follow-ups: ↓ 46 ↓ 52 Changed 8 years ago by 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?

I have a different comment.

```sage: plot(sqrt(x^2-1), -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 8 years ago by kcrisman

• Reviewers changed from Ralf Stephan to Ralf Stephan, Karl-Dieter Crisman

### comment:46 in reply to: ↑ 44 ; follow-up: ↓ 49 Changed 8 years ago by rws

```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 up-to-date.

### comment:47 Changed 8 years ago by git

• 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 8 years ago by ppurka

@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 ; follow-up: ↓ 50 Changed 8 years ago by kcrisman

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 up-to-date.

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 ; follow-up: ↓ 51 Changed 8 years ago by ppurka

I clearly don't get the wonders of git yet. Sigh.

May I suggest my write up `;-)`

### comment:51 in reply to: ↑ 50 Changed 8 years ago by kcrisman

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 8 years ago by kcrisman

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 user-provided 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 'auto-excluded 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^2-1)), (x, 0, 2*pi), exclude=[2,3])
sage: polar_plot(sin(sqrt(x^2-1)), (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^2-1),sqrt(x^2-1/2)),(x,0,5), exclude=[2,3])
sage: parametric_plot((sqrt(x^2-1),sqrt(x^2-1/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 8 years ago by ppurka

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 8 years ago by git

• 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 8 years ago by ppurka

I am not clear how to fix the parametric plot. I can look into it in detail only over the weekend.

### comment:56 Changed 8 years ago by git

• 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 8 years ago by git

• 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 8 years ago by ppurka

I think I have fixed the problem with the parametric and polar plots. Needs review. :)

### comment:59 follow-up: ↓ 60 Changed 8 years ago by kcrisman

• 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 8 years ago by ppurka

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 8 years ago by vbraun

• 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/site-packages/sage/doctest/forker.py", line 480, in _run
self.execute(example, compiled, test.globs)
File "/home/release/Sage/local/lib/python2.7/site-packages/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/site-packages/sage/misc/decorators.py", line 550, in wrapper
return func(*args, **options)
File "/home/release/Sage/local/lib/python2.7/site-packages/sage/plot/plot.py", line 1659, in parametric_plot
return plot(funcs, *args, **kwargs)
File "/home/release/Sage/local/lib/python2.7/site-packages/sage/misc/decorators.py", line 705, in wrapper
return func(*args, **kwds)
File "/home/release/Sage/local/lib/python2.7/site-packages/sage/misc/decorators.py", line 550, in wrapper
return func(*args, **options)
File "/home/release/Sage/local/lib/python2.7/site-packages/sage/plot/plot.py", line 1176, in plot
G = _plot(funcs, *args, **kwds)
File "/home/release/Sage/local/lib/python2.7/site-packages/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 8 years ago by ppurka

This shouldn't happen at all. This is what I fixed in #16439. Does this happen if you apply this ticket after #16439?

### comment:63 Changed 8 years ago by ppurka

hmm.. git is not merging the two tickets correctly. I will have to check.

### comment:64 Changed 8 years ago by git

• 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 8 years ago by ppurka

• 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.

Last edited 8 years ago by ppurka (previous) (diff)

### comment:66 Changed 8 years ago by ppurka

• 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 8 years ago by ppurka

• 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 8 years ago by kcrisman

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 8 years ago by vbraun

• Status changed from needs_review to positive_review

### comment:70 Changed 8 years ago by vbraun

• 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
Note: See TracTickets for help on using tickets.