Form refactor #588

rocky · 2022-10-30T04:05:15Z

This is basically all the time I could work on this, this weekend.

My goal here in this branch and commit is to start a discussion around how Boxing could be driven based on Form and
to start to decouple formatting/rendering from boxing better.

This should also make our MakeBoxes[] conformance with WMA better, and easier to track.

Right now, lower-level formatting and boxing are too intertwined: now we cannot separate these formatting and boxing.

Our MakeBoxes output is incorrect, and this is shown in running formatest.py (which won't be in a final PR, but will instead be worked into a pytest):

python formtest.py
expression:         a ^ b
Parsed expression:  System`Power[Global`a, Global`b]
Mathics MakeBoxes:  System`MakeBoxes[System`Power[Global`a, Global`b], System`StandardForm]
Eval'd Makeboxes:   RowBox[List[(<String: "Power">, <String: "[">, RowBox[List[(<String: "a">, <String: ",">, <String: "b">)]], <String: "]">)]]
New    MakeBoxes:   System`SuperscriptBox[Global`a, Global`b]
------------------------------

expression:         Sqrt[a]
Parsed expression:  System`Sqrt[Global`a]
Mathics MakeBoxes:  System`MakeBoxes[System`Sqrt[Global`a], System`StandardForm]
Eval'd Makeboxes:   <mathics.builtin.box.layout.SqrtBox object at 0x7ff41a6dadf0>
New    MakeBoxes:   System`Sqrt[Global`a]
------------------------------

expression:         a ^ (1/2)
Parsed expression:  System`Power[Global`a, System`Times[1, System`Power[2, -1]]]
Mathics MakeBoxes:  System`MakeBoxes[System`Power[Global`a, System`Times[1, System`Power[2, -1]]], System`StandardForm]
Eval'd Makeboxes:   RowBox[List[(<String: "Power">, <String: "[">, RowBox[List[(<String: "a">, <String: ",">, RowBox[List[(<String: "Times">, <String: "[">, RowBox[List[(<String: "1">, <String: ",">, RowBox[List[(<String: "Power">, <String: "[">, RowBox[List[(<String: "2">, <String: ",">, <String: "-1">)]], <String: "]">)]])]], <String: "]">)]])]], <String: "]">)]]
New    MakeBoxes:   System`SqrtBox[Global`a]
------------------------------

In the above, the output under New MakeBoxes more closely matches what WMA produces. Notice that neither the expression output currently of "Mathics MakeBoxes" nor that expression eval'd is close to correct.

Also, we don't right now have something that starts off with say System`FractionBox[1, Global`x] and just formats or renders this without adding more boxing junk.

Please review and let me know your thoughts.

rocky · 2022-10-30T04:18:43Z

In working on the above, I'd also like to share how my thoughts have changed since https://github.com/Mathics3/mathics-core/pull/585/files#r1003972475.

I think the biggest change is that I now think we should start out more rigidly not trying to intermingle boxing and formatting in one pass. It is possible WMA does indeed intermingle boxing and formatting - MakeBoxes does not accept a form that is not in $BoxForms. It is possible that were one to be able to look in WMA at lower-level that these are separate. I don't really know.

But for our purposes, I think we really need to start simple. Very simple. I am not suggesting we allow MakeBoxes to allow forms outside of $BoxForms, but internally we probably want a way to trace what is going on at a lower level.

From a code organization standpoint, and from the standpoint of understanding and debugging what is going on, separating the two will make things easier. Later on, we might find it convenient to intermingle. (Performance aspects here should not be considered. We have to first get things right before we get them fast.)

You will see that the methods in this PR work off slightly different types and in that link above. In particular, we need to use Form symbols and Elements rather than Form classes, and Element types which were used in that other link.

And you may see that I have given up on even attempting formatting code, e.g. numbers.py, because I now think we really need to just first focus on the Boxing aspect. Once that is done, then I think the formatting aspect will follow pretty simply.

mmatera · 2022-10-30T14:45:38Z

@rocky,

OK, this is getting huge. I will try to review this today.

mmatera · 2022-10-30T14:57:53Z

mathics/builtin/forms/default_boxing.py

+    return Expression(SymbolSqrtBox, *expr.elements)
+
+
+# More general, should we iterate over $BoxForms?


I think this is not necesary. To populate $BoxForms and make it effective, you should set the ParentForm to the new "BoxForm" :
https://reference.wolfram.com/language/ref/message/ParentForm/deflt.html

So, if MakeBoxes receives a form not in StandardForm, TraditionalForm as a second argument, then it should look at the parent form of this new form.

mmatera · 2022-10-30T15:00:33Z

mathics/builtin/statistics/dependency.py

 SymbolCovariance = Symbol("Covariance")
+
+# Something is weird here. No System`. And we can't use what is in
+# SymbolSqrt from systemsymbols?


Indeed, is the same symbol. Please just refer it from mathics.core.systemsymbols

I had tried replacing this and got an error. (I did not save nor do I remember the error).

I think what is different here is that instances of SqrtBox have some sort of dimension information and somehow (but I don't know why or how) that makes a difference.

mmatera · 2022-10-30T15:03:04Z

mathics/builtin/forms/base.py

+# should not get added as a definition (and therefore not added to
+# to external documentation.
+
+DOES_NOT_ADD_BUILTIN_DEFINITION = [FormBaseClass]


Maybe skipping any module named base is a better way to avoid loading "base" builtins than my idea of looking at a list.

There are already too many implicit conventions that we have. There is a principle that explicit is better than implicit.

Although this solution is not ideal — in devel I had this line in the wrong file and additional stuff was disappearing — for now let's leave it.

mmatera

LGTM. Let's merge and iterate.

mmatera · 2022-10-30T15:15:19Z

mathics/builtin/forms/base.py

+        Specific and custom methods need to be implemented for each Form
+        and element_type that perform some kind of boxing.
+        """
+        method = cls.form_box_methods.get((cls, element.head), None)


There is something that I am not seeing in this mechanism: Suppose that we set inside a session a makeboxes rule. Let's say

MakeBoxes[F[x_],f_]:=SubscriptBox[RowBox[{"<<",ToString[x],">>"}], "F"]

Where is this new rule stored?

How MakeBoxes find this rule?

There is something that I am not seeing in this mechanism: Suppose that we set inside a session a makeboxes rule. Let's say

MakeBoxes[F[x_],f_]:=SubscriptBox[RowBox[{"<<",ToString[x],">>"}], "F"]

Where is this new rule stored?

How MakeBoxes find this rule?

I am glad you asked this question. Let me direct or attention to this comment in the demo program.

This code is a sketch. It does not change MakeBoxes. Instead the goal was to give an organization where boxing, and later, formatting decisions have no notion of the form that they are embedded in.

It is possible and possibly likely that instead of doing this in methods it needs to be done as rules. In that case, instead of the box_divide(), box_power() methods inside mathics.builtin.forms.default_boxing, there would be rules instead. Or maybe this could be totally written as default_boxing_rules.m and autoloaded.

Here is a challenge for you if you are up for it...

Using MakeExpression, MakeBoxes and Format, write a new form like MMateraForm in WL code. It would simulate one of the non-trivial forms like StandardForm.

You are free to "cheat" (actually, you are encouraged to) and use rules from FormatValues or any information that a WL implementation will tell you.

I am glad you asked this question. Let me direct or attention to this comment in the demo program.

OK, I saw that. What I was trying to ask about is how you are envisioning that this could work.

This code is a sketch. It does not change MakeBoxes. Instead the goal was to give an organization where boxing, and later, formatting decisions have no notion of the form that they are embedded in.

It is possible and possily likely that instead of doing this in methods it needs to be done as rules. In that case, instead of the box_divide(), box_power() methods inside mathics.builtin.forms.default_boxing, there would be rules instead. Or maybe this could be totally written as default_boxing_rules.m and autoloaded.

Here is a challenge for you if you are up for it...

Using MakeExpression, MakeBoxes and Format, write a new form like MMateraForm in WL code. It would simulate one of the non-trivial forms like StandardForm.

You are free to "cheat" and use rules from FormatValues or any information that an WL implementation will tell you.

OK, I could provide you with some examples of that.

One other I hope small request here. Please start out with simple rules and Forms that lead to simplicity. There is this tendency to lead off with complicated and convoluted edge-case examples involving features we don't currently support in Mathics. And then we go down a rabbit hole focusing on this which doesn't change the base situation.

Instead, it would be awesome if we could start out with simple rules or Forms mimics that cover the majority of the common cases.

This is not to say that we won't get to the complicated and convoluted cases, just that we might do that later after we have some simple mastery of doing things more closely as it is done in Mathematica. (In general, I'd say this is a good thing to practice doing anyway.)

In particular we separate: * top-level forms in $OutputForms, * top-level forms not in $OutputForms * form variables $OutputForms, $PrintForms * base class code This is the non-experimental part of #588

A number of details of this are wrong - we probably want to hook into rules rather than write Python methods.

rocky requested review from TiagoCavalcante and mmatera October 30, 2022 04:05

rocky force-pushed the Form-refactor branch 2 times, most recently from cb1bcf9 to 5d226c9 Compare October 30, 2022 05:32

mmatera reviewed Oct 30, 2022

View reviewed changes

mmatera approved these changes Oct 30, 2022

View reviewed changes

mmatera reviewed Oct 30, 2022

View reviewed changes

rocky marked this pull request as draft October 30, 2022 16:24

rocky mentioned this pull request Nov 1, 2022

Split out builtin/forms.py into a module #589

Merged

Sketch to showing how to drive Box from Form

633e8c8

A number of details of this are wrong - we probably want to hook into rules rather than write Python methods.

rocky force-pushed the Form-refactor branch from 071da92 to 633e8c8 Compare November 1, 2022 12:42

mmatera added 4 commits November 21, 2022 18:56

Merge branch 'master' into Form-refactor

e8b20b2

Merge branch 'master' into Form-refactor

d4a9801

Merge branch 'master' into Form-refactor

25e90e6

Merge branch 'master' into Form-refactor

8cf2d31

		return Expression(SymbolSqrtBox, *expr.elements)


		# More general, should we iterate over $BoxForms?

Uh oh!

Form refactor #588

Are you sure you want to change the base?

Form refactor #588

Uh oh!

Conversation

rocky commented Oct 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rocky commented Oct 30, 2022

Uh oh!

mmatera commented Oct 30, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmatera left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rocky Oct 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rocky commented Oct 30, 2022 •

edited

Loading

rocky Oct 30, 2022 •

edited

Loading