Open marcelwa opened 2 years ago
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 :pray:
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!