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

Simplify definition of nodes #165

Closed
jakobj opened this issue Jun 30, 2020 · 1 comment · Fixed by #178
Closed

Simplify definition of nodes #165

jakobj opened this issue Jun 30, 2020 · 1 comment · Fixed by #178
Labels
enhancement New feature or request
Milestone

Comments

@jakobj
Copy link
Member

jakobj commented Jun 30, 2020

There is a lot of duplicate code accumulating in node.py. Can we find a simpler, more compact way to define node operations that can be translated to the different compilation options (numpy, sympy etc) behind the scenes? It would be great if the definition of, for example, the addition and parameter nodes would reduce to this:

class Add(Node):
    _output = "x_0 + x_1"

class Parameter(ParametrizedNode):
    _parameters = "p"
    _output = "p"

It's possible that we'll need to introduce new subclasses of Node such as UnaryNode, BinaryNode etc to implement this cleanly.

This would also allow us to easily add new compilation options without touching the definition of every single Node subclass.

@jakobj jakobj added enhancement New feature or request help wanted Extra attention is needed labels Jun 30, 2020
@jakobj jakobj added this to the 0.2.0 milestone Jun 30, 2020
@mschmidt87
Copy link
Member

I think it would be really nice to organize the Node classes into a hierarchical structure. I am not sure how to realize a scenario from your code snippet, though. How would these strings be parsed?

@jakobj jakobj removed the help wanted Extra attention is needed label Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants