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

Improve interface for symbolic constants #44

Closed
MasonProtter opened this issue Aug 26, 2018 · 6 comments · Fixed by #45
Closed

Improve interface for symbolic constants #44

MasonProtter opened this issue Aug 26, 2018 · 6 comments · Fixed by #45
Labels
improvement Improvement to existing feature

Comments

@MasonProtter
Copy link
Contributor

I noticed today that the following does not behave as I'd expect:

julia> Rewrite.normalize((@term 1 + x + 3), @term RULES [1 + x => y])
@term(y)

this should return @term(y + 3). Did I do something in defining the rule?

@HarrisonGrodin
Copy link
Owner

HarrisonGrodin commented Aug 26, 2018

There are two issues with this scenario:

First, the rule doesn't quite make sense - it says that (1 + an arbitrary value x) should be rewritten to (an arbitrary value y). Having variables on the right side that don't appear on the left should probably be an error, actually. This can currently be resolved though the use of constant functions, namely [x() + 1 -> y()].

Second, you're right that the functionality for normalizing subexpressions of flat terms is currently broken - see #21.

@MasonProtter
Copy link
Contributor Author

Re point 1) I was careless there because x and y were indeed arbitrary. What I often want instead is to literally replace all instances of the variable x with instances of the variable y. Ie.

julia> x, y = Variable.([:x, :y])
2-element Array{Variable{TypeSet{Any}},1}:
 @term(x)
 @term(y)

julia> Rewrite.normalize((@term f(1 + 2*$x)), @term RULES [$x => $y]) == @term f(1 + 2 * y)
false

which I would argue is still not desirable behaviour. Such replacement rules are well supported in environments like Mathematica and I think would be valuable to support here, ie.
screen shot 2018-08-26 at 6 07 33 pm

@MasonProtter
Copy link
Contributor Author

In Mathematica pattern matching there is a distinction between

Stuff /. {x -> y}

and

Stuff /. {x_ -> y}

The first will match only on the literal symbol x, whereas the second will match anything. It'd be nice if the pattern matching syntax for Rewrite supported such a distinction as easily.

@HarrisonGrodin
Copy link
Owner

HarrisonGrodin commented Aug 27, 2018

The current way to deal with this is to use a constant function x()to reference the literal symbol, as done by most existing term rewriting literature. However, I definitely agree that this both looks and feels awkward, and I've been working on a more Julian API for term construction and storage in general over the past week. Should have a PR open soon.

@HarrisonGrodin HarrisonGrodin changed the title Spurious behaviour with custom rules Improve interface for symbolic constants Aug 27, 2018
@HarrisonGrodin HarrisonGrodin added the improvement Improvement to existing feature label Aug 27, 2018
@MasonProtter
Copy link
Contributor Author

My experience with term rewriting is pretty much limited to my experience with Mathematica, but for what its worth, I've noticed that Mathematica makes no distinction between function heads ie x() and regular symbols x. Would it be naïve to think that maybe we can just straightforwardly port the behaviour from constant functions to other symbols or find some other way to lessen the distinction (if desirable)?

@HarrisonGrodin
Copy link
Owner

HarrisonGrodin commented Aug 27, 2018

Nope, not at all - that's exactly what I did. 😄 Just opened #45, in fact - it is a WIP, but any feedback/comments you could give would be appreciated. Should have a major impact on this and a handful of other open issues.

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

Successfully merging a pull request may close this issue.

2 participants