-
Notifications
You must be signed in to change notification settings - Fork 143
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
Is it intended that create_pi()
does not trigger on_add
events?
#568
Comments
So I just discussed with Heinz about this. As I guessed, it was not a conscious decision, but has simply evolved with needs. We could, of course, re-discuss again in the future if there are more cases where calling |
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 |
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. |
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 theircreate_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 indepth_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 thoughon_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 thecreate_pi
function and manually callon_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 triggeron_add
events oncreate_pi()
because it'd then process PIs twice.Any input on this matter is welcome. Many thanks in advance!
The text was updated successfully, but these errors were encountered: