Skip to content

leaf -> element more places#168

Merged
rocky merged 1 commit intomasterfrom
leaves-to-elements-part3
Feb 22, 2022
Merged

leaf -> element more places#168
rocky merged 1 commit intomasterfrom
leaves-to-elements-part3

Conversation

@rocky
Copy link
Copy Markdown
Member

@rocky rocky commented Feb 21, 2022

Side Note: Tracing Add[1, 2] seems to do a lot of unnecessary data motion and conversion back and forth.

We convert to from a tuple to a list, evaluate elements that are already in their reduced form, try to flatten a flattened list convert that into a Mathics Sequence only to convert it back to something like what it was before.

The lack of comments/commentary about what is done and why and lack of modularity (which I really still don't know how to deal with) just is obscuring this convoluted process. Sure, in some cases it needs to be this complex. Just not all of the time.

@rocky rocky force-pushed the leaves-to-elements-part3 branch 3 times, most recently from 6374091 to 3335de3 Compare February 21, 2022 20:34
Copy link
Copy Markdown
Contributor

@TiagoCavalcante TiagoCavalcante left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread mathics/core/atoms.py Outdated

def evaluate(self, evaluation) -> "Number":
"""Evaluation of a Number is just itself"""
# Why bother checking timout? If there is one
Copy link
Copy Markdown
Contributor

@TiagoCavalcante TiagoCavalcante Feb 21, 2022

Choose a reason for hiding this comment

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

Also: evaluating a number is (should be) instantaneous, so it is probably not the best place to evaluation.check_stoped.

@TiagoCavalcante
Copy link
Copy Markdown
Contributor

@rocky this remembers me that Plus should be FinalForm (I don't remember the name), but its algorithm has a flaw that doesn't put terms together in just one evaluation.

@rocky
Copy link
Copy Markdown
Member Author

rocky commented Feb 21, 2022

@rocky this remembers me that Plus should be FinalForm (I don't remember the name), but its algorithm has a flaw that doesn't put terms together in just one evaluation.

I don't understand exactly what you mean. Maybe though it would be better to open an issue and give a detailed example.

@rocky rocky force-pushed the leaves-to-elements-part3 branch from 3335de3 to cb6bee9 Compare February 21, 2022 23:45
Revise rewrite_apply_eval comment for threading.
@rocky rocky force-pushed the leaves-to-elements-part3 branch from cb6bee9 to f754b46 Compare February 21, 2022 23:46
@TiagoCavalcante
Copy link
Copy Markdown
Contributor

I don't understand exactly what you mean. Maybe though it would be better to open an issue and give a detailed example.

I'll make a PR this weekend fixing this. In the Plus evaluation, we put together multiples of symbols (e.g.: y + 2y + x = 3y + x). But the Plus code put together just some variables, so something like this (but with way more terms) happen: Plus[a, b, c, a, b] -> Plus[2a, b, c, b] -> 2a + 2b + c

@rocky rocky merged commit 400009a into master Feb 22, 2022
@TiagoCavalcante TiagoCavalcante deleted the leaves-to-elements-part3 branch February 22, 2022 00:34
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

Successfully merging this pull request may close these issues.

2 participants