Pytest mathics.core.evaluators

[mmatera]

• apply_N-> eval_N and apply_nvalues -> apply_N which is closer to the convention.
• adding some pytests.
• Adding comments and improving doctrings.

Also, some improvements in the compatibility in the behavior of SameQ and Expression.numerify were introduced, in order to implement the tests.

[mmatera]

@rocky, I was not sure about the change in the names, because it could produce certain confusion when merging other PRs. However, these names are closer to the convention. What do you think?

[rocky]

Note that evaluation has the last two parameters swapped. What's up here?

[mmatera]

The order of the parameters in apply_N (former apply_nvalues) is just the order that it had when it was the method N.apply_with_prec. On the other hand, the order in eval_N (former apply_N) is inverted because I wanted to put precision as an optional parameter, and then, it should go after evaluation, which is not optional.

I also used that to check if the names at some places were confused (because if it does, it will raise errors).

[rocky]

Thanks for the explanation. This however then points out that this irregular and confusing.

For now, I think apply_N should be called apply_N_with_prec. (And we should rename N.apply_with_prec to N.apply_N_with_prec as well.

I also used that to check if the names at some places were confused (because if it does, it will raise errors).

This might seem like a clever trick or hack, but it's really the wrong way to handle the problem. The right way is to add types to the arguments and use flake8 or pylint to catch type errors rather than making parameter ordering.

[mmatera]

Thanks for the explanation. This however then points out that this irregular and confusing.

For now, I think apply_N should be called apply_N_with_prec. (And we should rename N.apply_with_prec to N.apply_N_with_prec as well.

So, the idea would be to do not have prec as an optional argument, isn't it?

I also used that to check if the names at some places were confused (because if it does, it will raise errors).

This might seem like a clever trick or hack, but it's really the wrong way to handle the problem. The right way is to add types to the arguments and use flake8 or pylint to catch type errors rather than making parameter ordering.

Yes, but if both methods have the same parameters, flake8 and pylint are not able to distinguish which of the methods we need at some point of the code. But OK, I should have to add the types.

[rocky]

@rocky, I was not sure about the change in the names, because it could produce certain confusion when merging other PRs. However, these names are closer to the convention. What do you think?

I just took a look at apply_N() and I don't understand it or it looks like a big horrible hack.

In the docstring It is not clear where the "reaches" a fixed point part is. I suppose this is the implied behavior of any kind of the Mathics/WL rewrite-rules apply process.

The reason it doesn't feel to me like this clears things up yet, is because the whole apply/eval concept is too implicit and possibly inconsistent.

So let me back up to make sure everyone is on the same page with respect to what "apply" and "eval" mean in Lisp functional programming as described in SICP (Sussman's and Abelson's The Structure and Interpretation of Computer Programs).

This is where I am pretty sure Wolfram got his ideas from. He says he was following Lisp. Also, not that when I talk about "apply" I am not talking about Mathics/WL Apply[]. That is similar, but forget about that. It's not going to help here. Here we are concerned with the implementation of a WL interpreter written in Python.

In functional programming and Lisp particular (but also true to some extent in Python and other programming languages) suppose I write:

(add1 4)

This in Lisp is a list, just like (10 4) is a list.

It's "Head" (or in lisp "car") is add1. The process of noticing that add1 is a function and it takes argument is 4 is typically called the "apply" process: I "apply" list with one item (4) to the function add1.

In Lisp it is pretty easy to figure out what function to call and what it's arguments are. In WL this process is more involved with pattern matching and rule rewriting. Nevertheless I'll refer to this as the "apply" part, because that is totally analogous with what is done in Lisp. Any WL implementation has to do this kind of thing and so for lack of a better work "apply" seems to fit.

Now after I call add1(4) that is the evaluation part. So in this respect eval_N() is in my opinion the right thing.
In implementing the function add1() I may decide hey, let me create another list, maybe (add 1 4) and the call "apply+evaluate" or "Expression(add, 4).evaluate() " to both first apply to get a function with arguments and then evaluate that function with its parsed arguments to give a result.

So having clarified (hopefully) what apply and eval both mean this is where the ugliness in our current code lies in my opinion.

The apply step is what gets us to say apply_N() or apply_with_prec() of N. But once we get to that function its job is evaluation not application. And that is why those names in my opinion would have been better called eval_N() or eval_N_with_prec().

Because the word apply is used here incorrectly, using it correctly in N_apply() in this module adds confusion or inconsistency when it attempts to clear things up.

If we were to go through the monster process of changing those apply functions to eval, then N_apply() would make more sense.

My take to start out then to somehow document or comment we mean.

[mmatera]

I just took a look at apply_N() and I don't understand it or it looks like a big horrible hack.

Well, apply_N is just as hacky as it was before when it was named N.apply_with_prec, and yes, is something that needs some work on documenting, and probably reformulating. This PR is about picking the right names and add tests for these internal routines to establish their behaviors "by extension".

In the docstring It is not clear where the "reaches" a fixed point part is. I suppose this is the implied behavior of any kind of the Mathics/WL rewrite-rules apply process.

yep. And also, maybe this claim of "reaching a fixed point" is also wrong: there is not an iterative process like in Expression.evaluate, but something similar to the method Expression.evaluate_next. But again, the documentation here must be improved. One simpler statement for the docstring could be
"""
``apply_Ntakes an expressionexpr`, and looks for a numerical value with precision `prec` by applying the `NValue` rules stored in the `evaluation` object.
"""

The reason it doesn't feel to me like this clears things up yet, is because the whole apply/eval concept is too implicit and possibly inconsistent.

So let me back up to make sure everyone is on the same page with respect to what "apply" and "eval" mean in Lisp functional programming as described in SICP (Sussman's and Abelson's The Structure and Interpretation of Computer Programs).

This is where I am pretty sure Wolfram got his ideas from. He says he was following Lisp. Also, not that when I talk about "apply" I am not talking about Mathics/WL Apply[]. That is similar, but forget about that. It's not going to help here. Here we are concerned with the implementation of a WL interpreter written in Python.

In functional programming and Lisp particular (but also true to some extent in Python and other programming languages) suppose I write:

(add1 4)

This in Lisp is a list, just like (10 4) is a list.

It's "Head" (or in lisp "car") is add1. The process of noticing that add1 is a function and it takes argument is 4 is typically called the "apply" process: I "apply" list with one item (4) to the function add1.

In Lisp it is pretty easy to figure out what function to call and what it's arguments are. In WL this process is more involved with pattern matching and rule rewriting. Nevertheless I'll refer to this as the "apply" part, because that is totally analogous with what is done in Lisp. Any WL implementation has to do this kind of thing and so for lack of a better work "apply" seems to fit.

Now after I call add1(4) that is the evaluation part. So in this respect eval_N() is in my opinion the right thing.

Good!

In implementing the function add1() I may decide hey, let me create another list, maybe (add 1 4) and the call "apply+evaluate" or "Expression(add, 4).evaluate() " to both first apply to get a function with arguments and then evaluate that function with its parsed arguments to give a result.

So having clarified (hopefully) what apply and eval both mean this is where the ugliness in our current code lies in my opinion.

Yes, and I think this is also due to many of these aspects was not clear enough for the people working on the project.

The apply step is what gets us to say apply_N() or apply_with_prec() of N. But once we get to that function its job is evaluation not application. And that is why those names in my opinion would have been better called eval_N() or eval_N_with_prec().

well, here I tried to follow the logic that we have before: when evaluation_next found N[expr, prec] it, (after the leaves were evaluated) "applies" N.apply_with_prec and continue the evaluation. Now, N.apply_with_prec delegates the logic to apply_N. On the other hand, in many places of the code, we had the idiom
Expression("N", expr, prec).evaluate(evaluation)" to obtain a number from `expr`. Now this idiom is replaced by eval_N(expr, evaluation, prec)``
which I think has the following virtues:

• is more concise
• is a little bit faster, because avoids several function calls
• makes the code less entangled and easier to be analized.
• clarifies a concept about what an evaluation is in Mathics (at least now): take an expression, and and Evaluation object (that contains both a set of Definitions as well as certain other state variables) and produce a new Expression.

Because the word apply is used here incorrectly, using it correctly in N_apply() in this module adds confusion or inconsistency when it attempts to clear things up.

OK, so, what should be the name?

If we were to go through the monster process of changing those apply functions to eval, then N_apply() would make more sense.

I would prefer to not do that moster process.

My take to start out then to somehow document or comment we mean.

[rocky]

OK, so, what should be the name?

Because things are a bit confusing and there isn't in my opinion a clear path, my opinion is just leave things for later when it will become clearer how all of this should be addressed.

In my view efficiency we haven't achieved very effectively, although we have been very successful at making the code complicated and harder to understand. That's partly why I have been advocating no more speed improvements until we have a clearer idea of the big picture and a more modular and scalable structure and where we can measure and benchmark what's going on more effectively.

Whatever complexity is there right now (whether or not in the name of "efficiency") let's just leave as is. I think we'll have to come back to this later.

[mmatera]

This PR was reduced to a change of names. I close it and start again later.

[rocky]

I am always a little sad when people work on things here and it has to be redone because of other work.

Here though I'd say please hold off on any kind of evaluator/evaluation improvements for a while. We are in the process of reviewing all of this. So there is going to be an upheaval. here.

And there is plenty of other things to do. For example, #167 seems like a simple isolated little thing (based on the work that was done in Symja).

[mmatera]

No problem. Most of the work here was merged in another PR. What is remaining here was a proposal of name changes.

[rocky]

Relieved then.