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.

Sign up for GitHub

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

Improve directed infinity #809

Merged
merged 4 commits into from
Mar 12, 2023
Merged

Improve directed infinity #809

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9c6b54b
Improve rules for DirectedInfinity
mmatera Mar 6, 2023
8194c2c
adding tests
mmatera Mar 6, 2023
3415be9
Misc revisions mentioned in PR #809
rocky Mar 12, 2023
8f1dc7e
Merge pull request #812 from Mathics3/improve_DirectedInfinity-rocky
rocky Mar 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,25 @@
CHANGES
=======


API
Copy link
Member

@rocky rocky Mar 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually API refers the external API - things that a person using core would have to worry about.

I suspect this is just about internal organization and under the general category of we are moving evaluation functions into its own module, separate from the eval methods that function application finds.

---

* ``eval_sign`` and ``eval_abs`` are now in ``mathics.eval.arithmetic``.

Compatibility
-------------

* ``*Plot`` does not show messages during the evaluation.



Bugs
----

* Improved support for ``DirectedInfinity`` and ``Indeterminate``.


6.0.1
-----

Expand Down
124 changes: 74 additions & 50 deletions mathics/builtin/arithmetic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
Basic arithmetic functions, including complex number arithmetic.
"""

from mathics.eval.numerify import numerify
import sympy

# This tells documentation how to sort this module
sort_order = "mathics.builtin.mathematical-functions"
Expand All @@ -16,7 +16,6 @@
from functools import lru_cache

import mpmath
import sympy

from mathics.builtin.base import (
Builtin,
Expand Down Expand Up @@ -55,11 +54,9 @@
from mathics.core.symbols import (
Atom,
Symbol,
SymbolAbs,
SymbolFalse,
SymbolList,
SymbolPlus,
SymbolPower,
SymbolTimes,
SymbolTrue,
)
Expand All @@ -72,8 +69,11 @@
SymbolTable,
SymbolUndefined,
)
from mathics.eval.arithmetic import eval_mpmath_function
from mathics.eval.arithmetic import eval_abs, eval_mpmath_function, eval_sign
from mathics.eval.nevaluator import eval_N
from mathics.eval.numerify import numerify

ExpressionComplexInfinity = Expression(SymbolDirectedInfinity)


class _MPMathFunction(SympyFunction):
Expand Down Expand Up @@ -208,6 +208,13 @@ class Abs(_MPMathFunction):
summary_text = "absolute value of a number"
sympy_name = "Abs"

def eval(self, x, evaluation):
"%(name)s[x_]"
result = eval_abs(x)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should follow convention using capital letter. Also it would be nice to start removing %(name)s.

I will be putting in a PR to merge into this to correct such things.

if result is not None:
return result
return super(Abs, self).eval(x, evaluation)


class Arg(_MPMathFunction):
"""
Expand Down Expand Up @@ -590,8 +597,7 @@ class DirectedInfinity(SympyFunction):
= Indeterminate

>> DirectedInfinity[0]
: Indeterminate expression 0 Infinity encountered.
= Indeterminate
= ComplexInfinity

#> DirectedInfinity[1+I]+DirectedInfinity[2+I]
= (2 / 5 + I / 5) Sqrt[5] Infinity + (1 / 2 + I / 2) Sqrt[2] Infinity
Expand All @@ -602,15 +608,12 @@ class DirectedInfinity(SympyFunction):

summary_text = "infinite quantity with a defined direction in the complex plane"
rules = {
"DirectedInfinity[Indeterminate]": "Indeterminate",
"DirectedInfinity[args___] ^ -1": "0",
"0 * DirectedInfinity[args___]": "Message[Infinity::indet, Unevaluated[0 DirectedInfinity[args]]]; Indeterminate",
# "DirectedInfinity[a_?NumericQ] /; N[Abs[a]] != 1": "DirectedInfinity[a / Abs[a]]",
# "DirectedInfinity[a_] * DirectedInfinity[b_]": "DirectedInfinity[a*b]",
# "DirectedInfinity[] * DirectedInfinity[args___]": "DirectedInfinity[]",
# Rules already implemented in Times.eval
# "z_?NumberQ * DirectedInfinity[]": "DirectedInfinity[]",
# "z_?NumberQ * DirectedInfinity[a_]": "DirectedInfinity[z * a]",
# Special arguments:
"DirectedInfinity[DirectedInfinity[args___]]": "DirectedInfinity[args]",
"DirectedInfinity[Indeterminate]": "Indeterminate",
"DirectedInfinity[Alternatives[0, 0.]]": "DirectedInfinity[]",
# Plus
"DirectedInfinity[a_] + DirectedInfinity[b_] /; b == -a": (
"Message[Infinity::indet,"
" Unevaluated[DirectedInfinity[a] + DirectedInfinity[b]]];"
Expand All @@ -622,39 +625,59 @@ class DirectedInfinity(SympyFunction):
"Indeterminate"
),
"DirectedInfinity[args___] + _?NumberQ": "DirectedInfinity[args]",
"DirectedInfinity[0]": (
"Message[Infinity::indet,"
" Unevaluated[DirectedInfinity[0]]];"
"Indeterminate"
),
"DirectedInfinity[0.]": (
# Times. See if can be reinstalled in eval_Times
"Alternatives[0, 0.] DirectedInfinity[z___]": (
"Message[Infinity::indet,"
" Unevaluated[DirectedInfinity[0.]]];"
" Unevaluated[0 DirectedInfinity[z]]];"
"Indeterminate"
),
"DirectedInfinity[DirectedInfinity[x___]]": "DirectedInfinity[x]",
"a_?NumericQ * DirectedInfinity[b_]": "DirectedInfinity[a * b]",
"a_ DirectedInfinity[]": "DirectedInfinity[]",
"DirectedInfinity[a_] * DirectedInfinity[b_]": "DirectedInfinity[a * b]",
}

formats = {
"DirectedInfinity[1]": "HoldForm[Infinity]",
"DirectedInfinity[-1]": "HoldForm[-Infinity]",
"DirectedInfinity[-1]": "PrecedenceForm[-HoldForm[Infinity], 390]",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be getting numbers like 390 from the operator tables of Mathics scanner.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Times has Precedence 400, this needs to have a slightly lower value to ensure the parenthesis.

Copy link
Member

@rocky rocky Mar 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is something then incomplete here. What comes in contact with what? Is there a value in Robert's table that is correct here?

And finally, if there is something that needs to be adjusted from the normal value of some other operator, a clearer way to do that would be to use that value and add or subtract from it. Something like Precedence["Times"] - xxx

Copy link
Contributor Author

@mmatera mmatera Mar 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe what is missing is better documentation of what PrecedenceForm means.
https://reference.wolfram.com/language/ref/PrecedenceForm.html

In WMA, the right value for the precedence would be exactly the same as Times (400):

In[1]:= Times[b, PrecedenceForm[3 A ,400]]                                                                                                                                    

Out[1]= b (3 A)

In[2]:= Times[b, PrecedenceForm[3 A ,401]]                                                                                                                                    

Out[2]= b 3 A

In Mathics, this does not works in the same way:

In[1]:= Times[b, PrecedenceForm[3 A ,400]]                                                                                                                                    

Out[1]= b (3 A)

In[2]:= Times[b, PrecedenceForm[3 A ,401]]                                                                                                                                    

Out[2]= b (3 A)

In[2]:= Times[b, PrecedenceForm[3 A ,399]]                                                                                                                                    

Out[2]= b (3 A)

but

In[1]:= Times[b, PrecedenceForm[A ,400]]                                                                                                                                    

Out[1]= b A

In[2]:= Times[b, PrecedenceForm[3 A ,401]]                                                                                                                                    

Out[2]= b A

In[2]:= Times[b, PrecedenceForm[3 A ,399]]                                                                                                                                    

Out[2]= b (A)

So maybe we need to adjust this before worrying about the exact value for the precedence in this rule.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good and important information. Give me some time to study this and compare with the existing tables and Robert's tables.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice also that Robert's tables have several values depending on the context. I guess that this is related to the use of PrecedenceForm in some internal the format rules in WMA.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this is a much larger issue, and I think it is not going to be solved in the way proposed above, let's remove this change. I created #811 to capture the discussion above, and track this problem.

"DirectedInfinity[]": "HoldForm[ComplexInfinity]",
"DirectedInfinity[DirectedInfinity[z_]]": "DirectedInfinity[z]",
"DirectedInfinity[z_?NumericQ]": "HoldForm[z Infinity]",
"DirectedInfinity[z_]": "PrecedenceForm[z HoldForm[Infinity], 390]",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

390 - see above.

}

def eval(self, z, evaluation):
"""DirectedInfinity[z_]"""
if z in (Integer1, IntegerM1):
def eval_directed_infinity(self, direction, evaluation):
"""DirectedInfinity[direction_]"""
if direction in (Integer1, IntegerM1):
return None
if isinstance(z, Number) or isinstance(eval_N(z, evaluation), Number):
direction = (z / Expression(SymbolAbs, z)).evaluate(evaluation)
return Expression(
SymbolDirectedInfinity,
direction,
elements_properties=ElementsProperties(True, True, True),
)
return None
if direction.is_zero:
return ExpressionComplexInfinity

normalized_direction = eval_sign(direction)
# TODO: improve eval_sign, to avoid the need of the
# following block:
# ############################################
if normalized_direction is None:
ndir = eval_N(direction, evaluation)
if isinstance(ndir, (Integer, Rational, Real)):
if abs(ndir.value) == 1.0:
normalized_direction = direction
else:
normalized_direction = direction / Abs(direction)
elif isinstance(ndir, Complex):
re, im = ndir.value
if abs(re.value**2 + im.value**2 - 1.0) < 1.0e-9:
normalized_direction = direction
else:
normalized_direction = direction / Abs(direction)
else:
return None
# ##############################################

if normalized_direction is None:
return None
return Expression(
SymbolDirectedInfinity,
normalized_direction.evaluate(evaluation),
elements_properties=ElementsProperties(True, False, False),
)

def to_sympy(self, expr, **kwargs):
if len(expr.elements) == 1:
Expand Down Expand Up @@ -719,8 +742,8 @@ class Im(SympyFunction):

def eval_complex(self, number, evaluation):
"Im[number_Complex]"

return number.imag
if isinstance(number, Complex):
return number.imag

def eval_number(self, number, evaluation):
"Im[number_?NumberQ]"
Expand Down Expand Up @@ -990,8 +1013,8 @@ class Re(SympyFunction):

def eval_complex(self, number, evaluation):
"Re[number_Complex]"

return number.real
if isinstance(number, Complex):
return number.real

def eval_number(self, number, evaluation):
"Re[number_?NumberQ]"
Expand All @@ -1000,7 +1023,6 @@ def eval_number(self, number, evaluation):

def eval(self, number, evaluation):
"Re[number_]"

return from_sympy(sympy.re(number.to_sympy().expand(complex=True)))


Expand Down Expand Up @@ -1148,20 +1170,22 @@ class Sign(SympyFunction):
"argx": "Sign called with `1` arguments; 1 argument is expected.",
}

rules = {
"Sign[Power[a_, b_]]": "Power[Sign[a], b]",
}

def eval(self, x, evaluation):
"%(name)s[x_]"
# Sympy and mpmath do not give the desired form of complex number
if isinstance(x, Complex):
return Expression(
SymbolTimes,
x,
Expression(SymbolPower, Expression(SymbolAbs, x), IntegerM1),
)
result = eval_sign(x)
if result is not None:
return result
# return None

sympy_x = x.to_sympy()
if sympy_x is None:
return None
return super().eval(x, evaluation)
# Unhandled cases. Use sympy
return super(Sign, self).eval(x, evaluation)

def eval_error(self, x, seqs, evaluation):
"Sign[x_, seqs__]"
Expand Down
Loading