Skip to content

Commit b1a4bff

Browse files
authored
Use max_recursion 0 in _Plot. (#1523)
[Recent discussion](#1522 (comment)) suggests that the default for max_recursion in plotting functions should be 0 for faster performance and better rendering. Also, use Symbols over character strings more often in Plot options routines. (There is probably more that could be done here, but that's for some other time.)
1 parent afed316 commit b1a4bff

File tree

1 file changed

+40
-28
lines changed

1 file changed

+40
-28
lines changed

mathics/builtin/drawing/plot.py

Lines changed: 40 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
from mathics.core.symbols import Symbol, SymbolList
3030
from mathics.core.systemsymbols import (
3131
SymbolAll,
32+
SymbolAutomatic,
3233
SymbolBlack,
3334
SymbolEdgeForm,
3435
SymbolFull,
@@ -260,43 +261,53 @@ def eval(self, functions, x, start, stop, evaluation: Evaluation, options: dict)
260261

261262
# Mesh Option
262263
mesh_option = self.get_option(options, "Mesh", evaluation)
263-
mesh = mesh_option.to_python()
264-
if mesh not in ["System`None", "System`Full", "System`All"]:
264+
if mesh_option not in (SymbolNone, SymbolFull, SymbolAll):
265265
evaluation.message("Mesh", "ilevels", mesh_option)
266266
mesh = "System`None"
267+
else:
268+
mesh = mesh_option.to_python()
267269

268270
# PlotPoints Option
269271
plotpoints_option = self.get_option(options, "PlotPoints", evaluation)
270-
plotpoints = plotpoints_option.to_python()
271-
if plotpoints == "System`None":
272+
if plotpoints_option is SymbolNone:
272273
plotpoints = 57
274+
else:
275+
plotpoints = plotpoints_option.to_python()
273276
if not (isinstance(plotpoints, int) and plotpoints >= 2):
274277
evaluation.message(self.get_name(), "ppts", plotpoints)
275278
return
276279

277280
# MaxRecursion Option
278281
max_recursion_limit = 15
279282
maxrecursion_option = self.get_option(options, "MaxRecursion", evaluation)
280-
maxrecursion = maxrecursion_option.to_python()
283+
284+
# Investigate whether the maxrecursion value is optimal. Bruce
285+
# Lucas observes that in some cases, using more points and
286+
# decreasing recursion is faster and gives better results.
287+
# Note that the tradeoff may be different for Plot versus
288+
# Plot3D. Recursive subdivision in Plot3D is probably a lot
289+
# harder.
290+
maxrecursion = 3
291+
281292
try:
282-
if maxrecursion == "System`Automatic":
283-
maxrecursion = 3
284-
elif maxrecursion == float("inf"):
285-
maxrecursion = max_recursion_limit
286-
raise ValueError
287-
elif isinstance(maxrecursion, int):
288-
if maxrecursion > max_recursion_limit:
293+
if maxrecursion_option is not SymbolAutomatic:
294+
maxrecursion = maxrecursion_option.to_python()
295+
if maxrecursion == float("inf"):
289296
maxrecursion = max_recursion_limit
290297
raise ValueError
291-
if maxrecursion < 0:
298+
elif isinstance(maxrecursion, int):
299+
if maxrecursion > max_recursion_limit:
300+
maxrecursion = max_recursion_limit
301+
raise ValueError
302+
if maxrecursion < 0:
303+
maxrecursion = 0
304+
raise ValueError
305+
else:
292306
maxrecursion = 0
293307
raise ValueError
294-
else:
295-
maxrecursion = 0
296-
raise ValueError
297308
except ValueError:
298309
evaluation.message(
299-
self.get_name(), "invmaxrec", maxrecursion, max_recursion_limit
310+
self.get_name(), "invmaxrec", maxrecursion_option, max_recursion_limit
300311
)
301312
assert isinstance(maxrecursion, int)
302313

@@ -312,22 +323,23 @@ def check_exclusion(excl):
312323
return True
313324

314325
exclusions_option = self.get_option(options, "Exclusions", evaluation)
315-
exclusions = eval_N(exclusions_option, evaluation).to_python()
316326
# TODO Turn expressions into points E.g. Sin[x] == 0 becomes 0, 2 Pi...
317327

318-
if exclusions in ["System`None", ["System`None"]]:
328+
if exclusions_option in (SymbolNone, (SymbolNone,)):
319329
exclusions = "System`None"
320-
elif not isinstance(exclusions, list):
321-
exclusions = [exclusions]
330+
else:
331+
exclusions = eval_N(exclusions_option, evaluation).to_python()
332+
if not isinstance(exclusions, list):
333+
exclusions = [exclusions]
322334

323-
if isinstance(exclusions, list) and all( # noqa
324-
check_exclusion(excl) for excl in exclusions
325-
):
326-
pass
335+
if isinstance(exclusions, list) and all( # noqa
336+
check_exclusion(excl) for excl in exclusions
337+
):
338+
pass
327339

328-
else:
329-
evaluation.message(self.get_name(), "invexcl", exclusions_option)
330-
exclusions = ["System`Automatic"]
340+
else:
341+
evaluation.message(self.get_name(), "invexcl", exclusions_option)
342+
exclusions = ["System`Automatic"]
331343

332344
# exclusions is now either 'None' or a list of reals and 'Automatic'
333345
assert exclusions == "System`None" or isinstance(exclusions, list)

0 commit comments

Comments
 (0)