update Userguide and design doc for Node

Questions:

• How does "removeView" Node from the .hpp works? Form what i've seen, it removes the GraphView from the Node, but not the Node from the GraphView. The assymetry of information seem problematic
• getNodeDelta: does it only takes the Delta layer or all the layers between the Delta and the Node?
• Why is Node's addParent private and not public? How do we add parents in our Node?
• parentIsBackEdge: give the def of back edge with a little drawing because i like drawings

Linked MRs

aidge_core!231 (closed) (If you can, suggest a possible fix)

assigned to @operrin

changed title from update doc API for pybind, graphview, nodes and operator to update doc API for pybind of graphview, nodes and operator

changed the description

added Documentation 📖 Enhancement ⭐ StatusWork in Progress labels

mentioned in merge request aidge_core!231 (closed)

changed title from update doc API for pybind of graphview, nodes and operator to update Userguide and design doc for Node

changed the description

changed the description

changed the description

changed the description

added StatusReview Ready label and removed StatusWork in Progress label

added to epic &14

In void Aidge::Node::forward() if a forward function return false , it's removed to forward list?

In void Aidge::Node::backward() if a backward function return false , it's removed to backward list?

https://gitlab.eclipse.org/eclipse/aidge/aidge_core/-/blob/main/src/graph/Node.cpp?ref_type=heads#L100-122

What is the question?

I would like to know if this behavior is the one to specify? Should a log be written by the function to warn the user?

Yes, it is the behavior to specify. If I recall correctly, the goal is to remove Hooks once they have done their job.

@olivierbichler

Are you talking about the hook class that was removed? Is the behavior still relevant?

Hook class was removed but Hook mecanism is still present and used with the addBeforeForward, addAfterForward (same for Backward) methods.

A Hook is a function that is called on top of the forward/backward.

We decided to implement the hook system by creating a stack of functions to call instead of adding a Hook class (which was the first design we experimented).

Olivier made so that Hook returns a boolean to know if they should still be used or not.

Another thing that I may mention, hooks should not be used to modify the values of the input/output. We cannot enforce it by design but it is not the role of the hook functions. They should only be "watcher".

I don't see the use of void addChild(std::shared_ptr otherView, const IOIndex_t outId = IOIndex_t(0), std::pair<NodePtr, IOIndex_t> otherInId = std::pair<NodePtr, IOIndex_t>(nullptr, gk_IODefaultIndex));

Either you take the N-th input of the graph or you take directly a Node but here, we take a node from the graph, we check if it's an input and so on, seems overkill

same question in addChildView

Can you specify the question? I don't understand why it is overkill.

Also please use link to the code for better readability

I feel like we're risking some data overlapping here: if you give the Node to connect it as an input, you don't need the graph (as in some other addchild) while if you give the graph, no need to furthermore tell it's that node (if you had the node, use the other addchild) but just the graph's input index would suffice. Asking for both the graph and the Node require extra precautions as we need to check that the Node is indeed an input Node of the and so on

inline NodePtr &getParent(const IOIndex_t inId) { lacks security (we only check assert(inId != gk_IODefaultIndex); not inId < number of inputs

Bound checking is done by the std::vector::at method.

See https://en.cppreference.com/w/cpp/container/vector/at

Then no need for the assert as constexpr IOIndex_t gk_IODefaultIndex = std::numeric_limitsstd::uint16_t::max(); = 65535

(from what i've seen for std::uint16_t it's not -1)

Assets of void Aidge::Node::addParent(const std::shared_ptr other_node, const IOIndex_t inId) { seem in the wrong order

We should sort which functions are to be used directly and which should only be used in other functions (otherwise uninformed ussers will inadvertently break their model

You should provide list of such functions.

closed

mentioned in commit aidge_core@1288177e