Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assignments with (named) Patterns in the lhs #898

Open
mmatera opened this issue Aug 3, 2023 · 5 comments
Open

Assignments with (named) Patterns in the lhs #898

mmatera opened this issue Aug 3, 2023 · 5 comments

Comments

@mmatera
Copy link
Contributor

mmatera commented Aug 3, 2023

Description

Assignments with named patterns in the LHS are not properly handled.

Expected behavior

Consider an assignment expression of the form expr:F[x_,y_]:=G[HoldForm[expr]]
In WMA,

In[1]:=  expr:F[x_,y_]:=G[HoldForm[expr]]
In[2]:= F[1,2]
Out[2]= G[F[1,2]]

In Mathics,

In[1]:=  expr:F[x_,y_]:=G[HoldForm[expr]]
In[2]:= F[1,2]
Out[2]= G[F[Pattern[1, _], Pattern[2, _]]]

In a similar way, in builtin symbols, docstring patterns of eval_ methods, of the patterns expr:Expr[...] are not interpreted as named patterns but actually produce an error.

Additional context

This kind of pattern is used for example in CellsToTeX` to produce messages without requiring to rebuild an expression. For example,

functionCall:trackCellIndexProcessor[data:{___?OptionQ}] :=
	Module[{indexed, cellIndex, intype},
		{indexed, cellIndex, intype} =
			processorDataLookup[functionCall,
				data, {"Indexed", "CellIndex", "Intype"}
			];
		
		If[indexed,
			SetOptions[CellToTeX,
				"CurrentCellIndex" -> cellIndex,
				"PreviousIntype" -> intype
			]
		];
		
		data
	]

here, functionCall is used instead of rebuilding the expression:

				data, {"Indexed", "CellIndex", "Intype"}
			];

Another more interesting reason to fix this issue is related with a discussion we have some time ago with @rocky, regarding the reason for returning None when a builtin eval_ method "fails".

Suppose that you have an eval method like this:

class   SortedTwoParms 
     ....
     def eval(self, x, y, evaluation):
          """SortedTwoParms[x_,y_]"""
           if x.get_sort_key()>y.get_sort_key():
                  Expression(SortedTwoParms, y, x)
           return None

To avoid returning None we could write

class   SortedTwoParms 
     ....
     def eval(self, x, y, evaluation):
          """SortedTwoParms[x_,y_]"""
           if x.get_sort_key()>y.get_sort_key():
                  Expression(SortedTwoParms, y, x)
           return Expression(SortedTwoParms, x, y)

but this adds a lot of overhead both in rebuilding the expression and, in the rewrite- evaluation loop, to compare the expressions with sameQ.

Instead, if we could use a named pattern, we could write

class   SortedTwoParms 
     ....
     def eval(self, x, y, evaluation):
          """expr:SortedTwoParms[x_,y_]"""
           if x.get_sort_key()>y.get_sort_key():
                  Expression(SortedTwoParms, y, x)
           return expr

which avoids the overhead of rebuilding the expression, and makes the SameQ evaluation faster (because if is decided by an is comparison, instead of a recursive comparison of leaves).

Note aside: WMA rewrite-evaluation loop do not check "SameQ" but check something like the is to decide if the evaluation continues. For example, in WMA

In[1]:= F[x_,y_]:=F[x,y]                                                        

In[2]:= F[1,2]                                                                  

$IterationLimit::itlim: Iteration limit of 4096 exceeded.

Out[2]= Hold[F[1, 2]]

In[3]:= expr:G[x_,y_]:=expr                                                     

In[4]:= G[1,2]                                                                  

Out[4]= G[1, 2]

Notice that in the case of the definition of F, we reach the iteration limit because the result of evaluating F[1,2] is different from the original expression. In mathics, on the other hand,

In[1]:= F[x_,y_]:=F[x,y]
Out[1]= None

In[2]:= F[1,2]
Out[2]= F[1, 2]

because the recursion is finished by the costly sameQ evaluation.

In any case, named patterns in the doctest strings do not work either.

@mmatera
Copy link
Contributor Author

mmatera commented Aug 3, 2023

Notice that for builtin rules, we have a hack to pass the original expression (see for example StringTrim.eval_pattern) but it is a hack, and it would be good to remove it in favor of a proper handling of named Pattern patterns.

@rocky
Copy link
Member

rocky commented Aug 6, 2023

Let me see if I can simplify the demonstration of the bug.

In WMA:

In[1]:= expr:F[x_] := G[HoldForm[expr]]                                         
In[2]:= ?F                                                                      
Out[2]= Global`F
        expr:F[x_] := G[HoldForm[expr]]

In Mathics3:

In[1]:= expr:F[x_] := G[HoldForm[expr]]
Out[1]= None
In[2]:= ?F
Out[2]= Null 
(* This  above is wrong . Global`F should have the definition: expr:F[x_] := G[HoldForm[expr]] *)

@mmatera
Copy link
Contributor Author

mmatera commented Aug 6, 2023

It works too. However, it is also instructive to evaluate

In WMA,

In[3]:=F[2]
Out[3]= G[F[2]]

In Mathics

In[3]:=F[2]
Out[3]= G[F[Pattern[2, _]]]

so the rule seems to be stored in some place, but in a wrong way...

@rocky
Copy link
Member

rocky commented Aug 6, 2023

Now that I think I understand the problem, it looks to me that fixing this involves:

  • detecting patterns when they appear as a function in the left-hand side of a delayed assignment,
  • storing the corresponding rule properly in the definition of function symbol mentioned on the left-hand side, and
  • applying the rule properly in expression evaluation

A "disentangled" PR then should do just this, and no more.

And once this is fixed, I imagine it will work properly when used inside the docstring of an evaluation method for a built-in function.

Given that right now this does not work in these kinds of docstrings, it is not surprising that they are not currently used inside the code docstrings. In future PR, we can start introducing these possibly, simplifying code.

@mmatera
Copy link
Contributor Author

mmatera commented Aug 6, 2023

Probably a first step would be to improve the mathics.core.Definitions.get_tag_position to understand that
get_tag_position(Expression(Pattern, name, pat), name) when name !="SystemPattern" must be interpreted as get_tag_position(pat, name)`

and in the Assignment evaluation, when Pattern is in the lhs, and a tag is not given, set the tag based on the second argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants