Finish moving vectorized plot routines to pymathics.vectorizedplot

[mmatera]

• Remove vectorized plot routines from Mathics-core.
• Add tests for vectorizedplot module in Mathics-core
• classes and routines that are not specific for vectorization, like NumericArray are kept.

[mmatera]

@rocky, @bdlucas1, this PR implements what we discussed yesterday.

[rocky]

@rocky, @bdlucas1, this PR implements what we discussed yesterday.

LGTM

[bdlucas1]

@mmatera can you help me out by showing me the line of Python code I would need to load the module?

[mmatera]

@mmatera can you help me out by showing me the line of Python code I would need to load the module?

Sure. Please give me the link to the project where you use it.

[mmatera]

@mmatera can you help me out by showing me the line of Python code I would need to load the module?

Sure. Please give me the link to the project where you use it.

For example, in MathicsDjango, you can put a init.m file in the $ROOT_DIR/autoload with the line

LoadModule["pymathics.vectorizedplot"];

or any other setup command.

This file is then loaded by the line https://github.com/Mathics3/mathics-django/blob/8a94c365a3fb54e5e88299757247be49384d91b5/mathics_django/web/models.py#L39

[bdlucas1]

@mmatera thanks for the info on how to programmatically load the required module.

I applied that to the relevant repo (bdlucas1/math-plotting-demo) and things seem to work, as far as it goes. I haven't looked at the code in great detail, and I'm taking it on faith that the testing is still done properly. However a couple concerns:

The module doesn't seem to have the three new plotting functions that I submitted PRs for. I assume the intent is to put them there? Can that be done before this PR is merged so that it is still possible to run them (as I think those branches will no longer work once front ends like m3d are updated to use the module instead of the flag).

A more general concern: I noticed there's a bunch of code in the vectorized module that is duplicated with some small changes (e.g defaults for vectorized vs non-vectorized) from core (although maybe there are more differences, it's hard to tell). I think this is indicative of the breadth of the interface between the module and core, and the difficulty of defining and maintaining a stable API between the two, which I think will be an ongoing maintenance headache. (Of course that's just reiterating ground we already covered in the meeting, so I'm aware of your position.)

[mmatera]

@mmatera thanks for the info on how to programmatically load the required module.

I applied that to the relevant repo (bdlucas1/math-plotting-demo) and things seem to work, as far as it goes. I haven't looked at the code in great detail, and I'm taking it on faith that the testing is still done properly.

Tests are what they used to be, except now I have two "sessions", one that loads the vectorizedplot library, and the other that uses the "native" code in core. So the tests should be as good as the original ones.

However a couple concerns:

The module doesn't seem to have the three new plotting functions that I submitted PRs for. I assume the intent is to put them there? Can that be done before this PR is merged so that it is still possible to run them (as I think those branches will no longer work once front ends like m3d are updated to use the module instead of the flag).

For preparing the module, I poke the code from this branch: https://github.com/Mathics3/mathics-core/tree/all_vectorized_plot_PRs
where I merge all your PRs over the current state of master. If something is missing, we can pick from that branch.

A more general concern: I noticed there's a bunch of code in the vectorized module that is duplicated with some small changes (e.g defaults for vectorized vs non-vectorized) from core (although maybe there are more differences, it's hard to tell). I think this is indicative of the breadth of the interface between the module and core, and the difficulty of defining and maintaining a stable API between the two, which I think will be an ongoing maintenance headache. (Of course that's just reiterating ground we already covered in the meeting, so I'm aware of your position.)

We can move more evaluation code from the module to Mathics-core and vice versa. What I do not like about the code which is currently in master is the use of an environment variable to choose the branch of the plotting code to be used on each particular case. I am a physicist, not a system engineer, so in the end, I rely on @rocky and your opinions. But from my perspective, -someone who writes simulation programs in WMA and Python, and a code voyeur- the need for all this code duplication seems to be a design issue: common code for vectorized and non-vectorized plotting should be in functions that do not depend on which implementation we are using. These functions should belong to mathics-core, right?

@mmatera thanks for the info on how to programmatically load the required module.

I applied that to the relevant repo (bdlucas1/math-plotting-demo) and things seem to work, as far as it goes. I haven't looked at the code in great detail, and I'm taking it on faith that the testing is still done properly. However a couple concerns:

The module doesn't seem to have the three new plotting functions that I submitted PRs for. I assume the intent is to put them there? Can that be done before this PR is merged so that it is still possible to run them (as I think those branches will no longer work once front ends like m3d are updated to use the module instead of the flag).

A more general concern: I noticed there's a bunch of code in the vectorized module that is duplicated with some small changes (e.g defaults for vectorized vs non-vectorized) from core (although maybe there are more differences, it's hard to tell). I think this is indicative of the breadth of the interface between the module and core, and the difficulty of defining and maintaining a stable API between the two, which I think will be an ongoing maintenance headache. (Of course that's just reiterating ground we already covered in the meeting, so I'm aware of your position.)

[bdlucas1]

Sorry, my mistake. What is missing is that the tests I added in the PRs for the 3 new plotting functions are not in vec_tests.yaml in core. (When I didn't see those tests running I mistook that for meaning that the 3 new functions themselves were not in the module.) They are in the tests directory in the module, but do they belong there if the tests are being run from core?

What I do not like about the code which is currently in master is the use of an environment variable to choose the branch of the plotting code to be used on each particular case.

If it's the environment variable per se that you object to, it is not an essential part of the design - I don't believe I use it any more, using instead the plot.use_vectorized_plot global variable. Another mechanism that could be used is a global system setting (sorry, not sure if that's what it's called in WMA) - there's plenty of precedent for things like $Language, $MinPrecision, $Pre/$Post etc. that affect the behavior of built-in functions.

Regarding code duplication - ideally yes the default position should be that code shared between core and the module should stay in core instead of being duplicated into the module (as is the case with the module currently). But that means that the interface to that shared code in core becomes part of the stable (?!) API. The "surface area" of that API, and also its maturity (is this code that is likely to change?) is a key consideration for whether something is a good candidate for becoming a separate module. My gut feeling, not having spent any time looking into what such a stable API would look like (albeit being the author of that code) is that of course it can work as a module, but these issues are going to make that approach a maintenance headache.

[mmatera]

Sorry, my mistake. What is missing is that the tests I added in the PRs for the 3 new plotting functions are not in vec_tests.yaml in core. (When I didn't see those tests running I mistook that for meaning that the 3 new functions themselves were not in the module.) They are in the tests directory in the module, but do they belong there if the tests are being run from core?

Yes, you are right, I forgot to put here the new YAML file. I will add it later.

What I do not like about the code which is currently in master is the use of an environment variable to choose the branch of the plotting code to be used on each particular case.

If it's the environment variable per se that you object to, it is not an essential part of the design - I don't believe I use it any more, using instead the plot.use_vectorized_plot global variable.

If removing the environment variable is an improvement, this switch is something that we are not fully supporting in the next release of Mathics-core, because in many ways, the support is still incomplete:

• vectorized plots are not supported in the documentation or in the Django frontend.
• MakeBoxes rules/functions are still not compatible with WMA
• render functions from boxes to SVG/asymptote are not complete.
• NumericArrays are still not fully implemented (we can not create them from the interpreter).

Since @rocky wants to do a release in the next month, I do not see feasible to fix all of this (and other things that I am forgetting right now) before the release. So we can make progress in the module, and eventually move what makes more sense to mathics-core.

Another mechanism that could be used is a global system setting (sorry, not sure if that's what it's called in WMA) - there's plenty of precedent for things like $Language, $MinPrecision, $Pre/$Post etc. that affect the behavior of built-in functions.

Indeed, there are many mechanisms. But for this kind of big things, the mechanism in WMA is to put the functionality in packages. Packages that can be .m packages in native WL, or packages that use the C interface with MathLink

Regarding code duplication - ideally yes the default position should be that code shared between core and the module should stay in core instead of being duplicated into the module (as is the case with the module currently). But that means that the interface to that shared code in core becomes part of the stable (?!) API. The "surface area" of that API, and also its maturity (is this code that is likely to change?) is a key consideration for whether something is a good candidate for becoming a separate module. My gut feeling, not having spent any time looking into what such a stable API would look like (albeit being the author of that code) is that of course it can work as a module, but these issues are going to make that approach a maintenance headache.

Currently, most of the API required for putting symbols outside is relatively stable. Most of the changes that have been made now was related to MakeBoxes rules (required for this specific module), and I hope we do not need to do major changes for a while. Other packages like NetworkX and NaturalLanguage shares the same interface, and required not too much effort to keep them updated.

[rocky]

Since @rocky wants to do a release in the next month, I do not see feasible to fix all of this (and other things that I am forgetting right now) before the release. So we can make progress in the module, and eventually move what makes more sense to mathics-core.

Yes. I am trying to be a bit heads down on getting a release out. That is one reason I haven't been tracking this closely and probably won't until the release is out. We still have several steps to go through before release.

[rocky]

@mmatera thanks for moving the additional 3D tests here.