Page MenuHome

Adding Attribute Math Expression Node
Needs ReviewPublic

Authored by Rajesh Advani (rajeshja) on Jun 2 2021, 4:44 PM.
Tags
None
Tokens
"Like" token, awarded by chen._123."Love" token, awarded by Rehten."Love" token, awarded by lukastoenne."Love" token, awarded by hyyou."Love" token, awarded by weasel."Love" token, awarded by digim0nk."Love" token, awarded by franMarz."Love" token, awarded by mswf."Like" token, awarded by Leul.

Details

Summary

Description of the problem that is addressed in the patch.

The Attribute Math node currently allows the user to use mathematical functions with 1, 2 or 3 variables, and combine multiple such functions together using multiple instances of the same node.
However, a mathematical operation with multiple operators and functions - like (x * sin(x)^2 - b)*c - requires at least 4 nodes to implement, and is hard to grok in a node tree with multiple such operations.

Description of the proposed solution, and a motivation as to why this is the best solution.

The proposed solution adds an Attribute Math Expression node, that accepts arbitrary mathematical expressions using one or more of the functions already available with the Attribute Math node. Additionally a "negate" operation has been added to support the unary minus operation.

I have implemented a simple version of the shunting yard algorithm with some modifications to support function calls and multi-parameter functions.

The expression parser has been abstracted externally, and is intended to evolve into a generic class that can use one of multiple possible parsing strategies, like vector math for example.
The new node offloads the user entered expression to this parser, and requests evaluation of it for each index of the span result. Any errors (exceptions) reported by the parser are relayed to the user using node errors.

The parser itself offloads the execution of individual operands and functions to NOD_math_functions.hh similar to how the Attribute Math node does it.

A similar approach can be used to create a generic Math Expression (the not-Attribute kind) node that can be used in multiple node editors like Geometry and Shader.

If support for new functions is added to blender, it is a matter of creating some additional configurations to leverage them in this parser.

Two constant names are currently supported - pi and e. Lower case only. This is because E needs to be recognised as the exponent in numbers like 1.23E-4.

The parser can identify any arbitrary variable names in the expression. The parse function currently doesn't report the variable names identified, but that is an easy change.

The evaluate function accepts values against the variables for evaluation.

The node itself however currently limits the variables that can be given values, to a, b and c.

So you can enter an expression like x * sin(x) but there is no way to give x a value. So you need to use a * sin(a) and give the A socket the corresponding value instead.

Eventually the intention is to be able to directly use attribute names in the expression (like index or position.x). It would be the node's responsibility to pass the values of these to the evaluate function.

The node code can also be updated to identify the number of variables in the expression, and show an appropriate number of input sockets with the right names. However, the user experience for that isn't clear to me. What would the initial input sockets be before an expression is entered?

List of alternative solutions, and a motivation as to why these are undesirable.

Parsing mathematical expressions isn't new, and there are multiple implementations of this available. Examples are libmatheval, other similar math calculators, expression parsers using antlr, etc. However, including these with blender would increase the dependencies on other libraries (antlr, yacc/lex, etc) increasing the build time and footprint of Blender which is long enough as it stands.

Limitations of the proposed solution.

Parsing of Hex and octal values is not supported right now.

I have currently not implement unit tests, as I haven't figure GTest out yet.

The parser is currently a monolith. However it is intended to be a generic implementation that operates against multiple strategies, the Math parser being the first implementation, followed by, say, the Vector Math parser.
Boolean operations are not supported yet. This could be added as another strategy.

The parser was not designed to support more generic expressions that include control functions (if/else, ternary, for, while, etc). The target was various kinds of math (floating point, Boolean, vector).

Given that it offloads evaluation of individual functions and operators in the expression to existing code in NOD_math_functions.hh, there is scope for optimization of the evaluate function.

I've dumbed the parser code down to use floats right now. It was originally coded using double. Ideally we'd use double for accuracy in calculations. However I'm not clear on the implication for nodes, since they currently seem to expose float values only (?). Also, all existing math uses float.

Mock-up of the proposed user interface and a description of how users are going to interact with the new feature.

A mockup is provided at:
https://blender.community/c/rightclickselect/Nshbbc/

Diff Detail

Repository
rB Blender
Branch
rja/exprnode (branched from master)
Build Status
Buildable 14901
Build 14901: arc lint + arc unit

Event Timeline

Rajesh Advani (rajeshja) requested review of this revision.Jun 2 2021, 4:44 PM
Rajesh Advani (rajeshja) created this revision.
Rajesh Advani (rajeshja) retitled this revision from Equation node initial state, not compiling. to Adding Attribute Math Expression Node.Jun 2 2021, 6:42 PM
Rajesh Advani (rajeshja) edited the summary of this revision. (Show Details)
Rajesh Advani (rajeshja) edited the summary of this revision. (Show Details)Jun 2 2021, 8:56 PM
Rajesh Advani (rajeshja) edited the summary of this revision. (Show Details)Jun 2 2021, 9:02 PM
Rajesh Advani (rajeshja) edited the summary of this revision. (Show Details)Jun 2 2021, 9:08 PM

First of all, thank you for working on Blender! This is impressive as a first patch.
Unfortunately, I couldn't really test it, because it crashes immediately in an ASAN build in this setup when I press enter (see P2152).

Generally, I do agree that we want to have an Attribute Expression node at some point (without "Math" in the name probably). However, I'd rather implement it differently.

  • This should use the same underlying execution system that will be used by the Attribute Processor that is being worked on (T85655). This way we can get much better performance compared to what you've implemented in this patch. Your ExpressionParser::evaluate has a lot of overhead for every operation and element. I've already spend quite some time thinking about and implementing ways that make this fast. I haven't worked on it for a while now, but now that we have the Attribute Processor on our agenda, I will hopefully have more time to further improve the system and to make it work better with expressions (I already implemented a parser + expression evaluator some time ago, but will need to rewrite it).
  • The Attribute Processor solves a slightly related problem to this patch. It makes building expressions with nodes easier compared to the current attribute workflow. Expressions can be added afterwards. Maybe we should first have an Expression node in the attribute processor before having an Attribute Expression node.
  • The expression evaluator should support more data types, ideally all data types we want. Not just floats. And it should support operations that deal with multiple types at the same time.
  • I haven't implementing the shunting yard algorithm myself yet, but it does not seem like a good fit. It's designed to convert infix to something else, which is a bit limiting. Personally, I found recursive descent parsers to work very well for parsing this kind of input.
  • Supporting a fixed number of inputs might be fine for now, not sure if it really is. Blender does not make it super easy to have a dynamic amount of sockets.
  • I do agree that we should not use an external library to parse these expressions. That is simple enough to implement ourself and gives us more control. We already have BLI_expr_pylike_parse, but I think we can build something better/more flexible (and hopefully use that new implemention to replace the existing parser).
  • Using floats instead of double was the right call. Generally, we use floats in Blender currently. Typically we only switch small parts to use doubles if there are specific precision issues.

For future updates/patches, please follow our code style (https://wiki.blender.org/wiki/Style_Guide/C_Cpp) and use clang-format (https://wiki.blender.org/wiki/Tools/ClangFormat).

@Jacques Lucke (JacquesLucke) First of all, thank you for the detailed response. It really helps explain the intended direction on this. Also, I'm really sorry about the crash on testing the code. I don't have a Mac, and was only able to test my patch on Windows. I'd love any suggestions to avoid such errors for future patches.

  1. I agree about the overhead in the evaluate method. There are multiple levels of indirection for each operation and element. However, the reason I didn't optimize it further was - (1) to keep the patch small, (2) to keep the implementation identical to individual nodes to allow comparative testing and (3) because it should be no slower than a nodetree using individual Attribute Math nodes, and so I thought that performance optimization could be done after getting the feature out. I would love to see any ideas you have on this, if you have them written down, or code that you have implemented even if it is old.
  2. The Attribute Processor seems like a neat step towards the same goal. And expressions would definitely be a way to implement more complicated combinations (e,g, sqrt(x*sin(x)^2+y*cos(x)^2).
  3. In the approach I went with, a boolean parsing strategy would exist side by side with a math strategy and a vector math strategy. I didn't think about mixing them up though. That's an interesting use-case, and I'd like to understand more about it.
  4. Recursive descent does seem better suited to this than shunting yard. I took a quick look and looks like BLI_expr_pylike_parse uses it?
  5. A simple way to introduce dynamic number of inputs would be to use a largish number (say, 6?) and then use a subset based on how many variable the user introduces.
  6. Is there something written down about creating a more flexible version of BLI_expr_pylike_parse? I could take a stab at it.
  7. The bit that bothers me about using floats is that I can't get sin(pi) to be exactly zero, or sin(pi/2) to be exactly 1. It's around 0.00001 instead. But the precision there might not be a priority.

I'd love to contribute to the Attribute Processor effort, so I'll be looking forward to any tasks that I can pick up on this.

Thanks again for taking a look at this.

I don't have a Mac, and was only able to test my patch on Windows. I'd love any suggestions to avoid such errors for future patches.

I was actually testing on Linux here, not Mac. I think Windows got support for Address Sanitizer (ASAN) recently as well, but I haven't used it on Windows yet.


  1. Keeping the patch small was no doubt a good idea. I wonder if you actually measured the statement "should be no slower than the nodetree using individual Attribute Math nodes". I just ran a quick test in the file linked below and measured that using individual attribute nodes for the expression a*2+b-c is about 50 times faster. I ran the with a single thread (starting Blender with --threads 1) for a more fair comparison. That performance difference is not too surprising to me tbh, there is really quite a lot of expensive overhead (memory allocations etc.). I did write a couple of documents on the topic the most recent one of which is this. Note that this is already a year old and some things have changed (implemention and planning wise). Maybe it's still interesting for you though. A lot of the code is already in master in the source/blender/functions folder.
  2. I think it should just be possible to have expressions like vec3(position.x, 3, length(direction)) (that's just a silly example of course, but it requires proper type handling.
  3. Didn't check it in detail now, but yes, it looks like a recursive descent parser. Here are two other recursive descent parsers that I've implemented in the last few years. One in C++ and one in Python. Note that none of them are production ready, they were just experiments.
  4. Yes that should work, but it's also a bit annoying..
  5. I don't have a great example right now, but just not hardcoding the allowed functions seems like a good step forward. I experimented with a symbol table here. And here are some tests.
  6. Doubles are not exactly 0 or 1 in these cases either unless they are implemented as special cases which I doubt. They do have better precision of course, but they are not perfect. It's just a trade off, and currently Blender uses mostly floats.

This is the benchmark file I used above:


You can change the Switch node to make it compute the expression using either the Expression node or Attribute Math nodes.
To get the execution time printed to the console I recommend putting SCOPED_TIMER(__func__); at the beginning of evaluate_geometry_nodes.

  1. Sorry, ignore the MacOS comment. I'm new to C++ and wasn't aware of ASAN. The first couple of search results mentioned XCode and I just assumed it was a Mac thing. Also, I only just noticed that P2152 is a link to the error log. I've booted into a Linux install now, and will try replicating the crash.
  2. My testing was outside of Blender. I extracted bits from Blender to compare my algorithm with what happens in Blender, but I was obviously doing something wrong. With your blend file and SCOPED_TIMER (thanks for this tip, it is massively useful), I can see that my code is between 50-200 times slower than the bigger node tree. I replaced my implementation with expr_pylike_eval and that is also 5-10x slower. But at least part of that is because I'm parsing the expression on every execute. I had originally planned on putting the parsed expression in an instance variable of the node object, and only parse when the expression changed, but still need to understand the blender code better before I figure out how to do that right.
  3. I get the picture about the multi-type example. Will keep it in mind.
  4. Thanks for the recursive descent parser examples examples. I'm trying to read up on the theory now.
  5. I have some ideas about the arbitrary sockets. Will test some stuff out.
  6. Symbol tables looks interesting. Will look into this.

I'll keep this patch going more for my personal interest. I am getting rid of most of ExpressionParser and replacing it with pylike for now. I'll keep the basic interface. I prefer the nodes (and any code that calls it) calling an abstraction than a specific implementation. Makes it easier to switch it out for something else later. Unless you think an added level of indirection adds significant overhead.

I have some design ideas for the Attribute Processor, and how it would play with expression parsing support. Is the Wiki the right place to document this? Or is that for committers only?

Thanks again for indulging me.

I have some design ideas for the Attribute Processor, and how it would play with expression parsing support. Is the Wiki the right place to document this? Or is that for committers only?

I recommend you just write it down somewhere and share it in the geometry nodes blender chat channel. Without more details about what your ideas are, I can't really say what's the right place. Personally, I'm typically taking notes in https://hackmd.io/ currently and share them in the geometry nodes channel as well.

Rajesh Advani (rajeshja) updated this revision to Diff 38283.EditedJun 14 2021, 5:30 PM
  • Removed extra line to get rid of it from patch submitted.
  • Added abstraction to switch between parsing implementations. Also added an implementation to use expr_pylike_eval.
  • Fixed memory access bug that was crashing ASAN.
  • Switched to PyLike parser implementation.

Got distracted by work, so took a little longer, but I've fixed the ASAN bug in the parser. And switched to using expr_pylike_eval as the parser. Switching to another parser (like my shunting yard implementation, which should be considered abandoned) in case there is any interest, would only require implementing the ExpressionParser interface and changing the ParserFactory::get_parser_instance() implementations to return the appropriate instances.

I don't yet know if throwing exceptions is an expensive error reporting mechanism, and whether it fits in with blender code design, so that's an area where the interface could be changed. Also, the current design doesn't account for the coming design of Attribute Processor.

Based on the comments from Jacques above, I think I would close this patch, since an expression system in geometry nodes should probably use the multi-function system as mentioned above.
There are other larger things like the need for a dynamic number of dynamically typed inputs to an expression node that make this a bit of a larger project.

I will leave that decision to Jacques though, since this is really his domain!


I have two more templates, you can refer to them, I hope it will appear on blender