Is it intended that `create_pi()` does not trigger `on_add` events?
Opened this issue · 5 comments
Describe the bug
First of all, I'm not sure whether this is a bug or intended behavior.
I have noticed that the default network implementations do not trigger the on_add events when their create_pi() function is called.
I found this behavior misleading as, e.g., depth_view makes explicit usage of this event type to recompute levels on creation of new nodes. There is a test case in depth_view.cpp that tests this feature which of course works because PIs are placed in level 0 anyway, i.e., the default-constructed value for the level data type (uint32_t). To be more precise, PIs show their intended level on creation even though on_add was never called on them.
I'm currently implementing a view that's similar to depth_view and I wanted to rely on this event to process new nodes (including PIs) on creation. @lee30sonia recommended to override the create_pi function and manually call on_add (many thanks for this idea!). While that provides intended behavior for my use case, it would break as soon as someone introduced a new network type that did trigger on_add events on create_pi() because it'd then process PIs twice.
Any input on this matter is welcome. Many thanks in advance!
So I just discussed with Heinz about this. As I guessed, it was not a conscious decision, but has simply evolved with needs.
I think (though I haven't tried) calling the on_add events in create_pi should not break any existing code. A drawback would be perhaps unnecessary overhead whenever create_pi is called. Thus, I would lean towards the solution I suggested (i.e. overloading create_pi in your view) for now.
We could, of course, re-discuss again in the future if there are more cases where calling on_add in create_pi would make sense.
Yes, from an architectural perspective that makes sense. Usually, though, the number of PIs should not be the dominating factor in a logic network compared to the number of nodes. Thus, I think the computational overhead of calling on_add on create_pi would be negligible. Either way, many thanks for your insights 🙏
Indeed, the overhead should not be too big, so I'm not totally against it. However, small overheads accumulate. The balance between pros and cons is not so clear to me at this point, so I tend to avoid fundamental changes to reduce inconvenience.
Totally agree with your points. Thanks a lot again. This issue can then be closed.
I will leave it open in case there will be more discussions from other cases/applications in the future, as I see this as an unsettled decision.