diff --git a/include/aidge/graph/GraphView.hpp b/include/aidge/graph/GraphView.hpp index 30c39f287955d379133d59f215022b346c2cbe6f..44c1942f5c23623d0a0f4cb783d9a3b2a7ea3e3e 100644 --- a/include/aidge/graph/GraphView.hpp +++ b/include/aidge/graph/GraphView.hpp @@ -44,7 +44,7 @@ enum class DataType; * - mNodes: The set of Nodes included in the GraphView * - : Set of nodes included in the graphview with names * - mInputNodes: GraphView inputs IOIndex_t designates the input number - * - mOutputNodes: GraphView outputs IOIndex_t designates the input number + * - mOutputNodes: GraphView outputs IOIndex_t designates the output number */ class GraphView : public std::enable_shared_from_this<GraphView> { private: @@ -304,14 +304,15 @@ public: DeviceIdx_t device = 0, const std::vector<std::vector<DimSize_t>> dims = {}); - /** @todo naming the input dims, while there exist currentTensorPtr->dims is a nice way to mess with reader's head maybe some proper variable name would be great - * @brief Compute dimensions of input/output Tensors for each Operator of the - * GraphView object's Nodes, by calling Node::forwardDims(). - * This function verifies the following conditions: - * - Every node will forwardDims() regardless of if dims were previously forwarded or not; - * - forwadDims() calls are made in node dependencies order, because if dims have changed + /** + * @brief Compute dimensions of input/output Tensors for each Node's + * Operator of the GraphView. + * @details This function verifies the following conditions: + * - Every node's dimensions will be updated regardless of if dims were previously forwarded or not; + * - Updates are made in node dependencies order, because if dims have changed * at any point in the graph, it must de propagated correctly to all succeeding nodes; * - It handles cyclic dependencies correctly (currently only induced by the Memorize_Op). + * @return bool Whether it succeeded or not (failure can either raise an exception or return false) */ bool forwardDims(const std::vector<std::vector<DimSize_t>>& dims = {}, bool allowDataDependency = false); @@ -332,10 +333,10 @@ public: /////////////////////////////////////////////////////// // TOPOLOGY /////////////////////////////////////////////////////// - //@todo 50 shades of get public: /** * @brief Get the parents Nodes of inputNodes of the graph. + * @details The parents are all put in a single set without ordering * @return std::set<NodePtr> */ std::set<NodePtr> getParents() const; @@ -345,21 +346,37 @@ public: * @return std::vector<NodePtr> */ std::vector<NodePtr> getParents(const std::string nodeName) const; + + /** + * @brief Get the parents Nodes of inputNodes of the graph. + * @details The parents are ordered in vectors, each vector containing the parents of an inputNode + * @return std::set<NodePtr> + */ std::vector<std::vector<NodePtr>> getOrderedParents() const; /** * @brief Get the children Nodes of outputNodes. + * @details The children are all put in a single set without ordering * @return std::set<NodePtr> */ std::set<NodePtr> getChildren() const; + /** * @brief Get children Nodes of the specified Node. + * @details the children will be ordered in a vector of vector of Node, + * each sub-vector containing the children from one of the Node's output * @param nodeName Name of the Node. * @return std::vector<std::vector<NodePtr>> */ std::vector<std::vector<NodePtr>> getChildren(const std::string nodeName) const; - std::set<NodePtr> getChildren( - const NodePtr otherNode) const; // TODO change it for a vector<vector> ? + + + /** + * @brief Get children Nodes of the specified Node. + * @param otherNode The Node + * @return std::vector<std::vector<NodePtr>> + */ + std::set<NodePtr> getChildren(const NodePtr otherNode) const; // TODO change it for a vector<vector> ? /** * @brief Get the Nodes in the GraphView. @@ -495,8 +512,12 @@ public: } /** - //todo only updates registery with pair (newName, node) but does not ensure node's name is indeed newName - * @brief + //todo only updates registery with pair (newName, node) but does not ensure node's name is indeed newName in its args + // todo this shouldn't be public but a friend + * @brief (from my comprehension) This function is made only to be used in Node's setName. + * This method update the registry with the new Node's Name + * @param node The Node that is updated + * @param newName The new name of the Node that shall soon be updated */ inline void updateNodeName(const std::shared_ptr<Node>& node, const std::string& newName){ if (!newName.empty()) { @@ -523,8 +544,7 @@ public: * Node, then it defaults to the first output Tensor of this Node. * @param toNode Pair of pointer to Node and Tensor ID for specifying the * connection. If the GraphView whose content is included has only one input - * Node, then it defaults to the first available data input Tensor of this - * Node. + * Node, then it defaults to the first available data input Tensor of this Node. */ void addChild(std::shared_ptr<GraphView> toOtherView, std::pair<NodePtr, IOIndex_t> fromOutNode = @@ -659,7 +679,7 @@ private: * inputs/outputs after adding this node. * @param nodePtr */ - //@todo 1see in code comment + //@todo see in code comment void updateInputsOutputsNew(NodePtr newNode); /** diff --git a/src/graph/GraphView.cpp b/src/graph/GraphView.cpp index c0c6ee59082e31f69533e6c53df4e2d0c9b87d72..d9aef04864310b2c7213229b1563f12cbfccfad2 100644 --- a/src/graph/GraphView.cpp +++ b/src/graph/GraphView.cpp @@ -437,8 +437,7 @@ bool Aidge::GraphView::forwardDims(const std::vector<std::vector<Aidge::DimSize_ std::size_t i = 0; for (auto& input : mInputNodes) { - const auto& currentTensorPtr = - std::dynamic_pointer_cast<OperatorTensor>(input.first->getOperator())->getInput(input.second); + const auto& currentTensorPtr = std::dynamic_pointer_cast<OperatorTensor>(input.first->getOperator())->getInput(input.second); if (i < dims.size() && !dims[i].empty()) { if (currentTensorPtr) { // tensor detected AIDGE_ASSERT(currentTensorPtr->dims() == dims[i], @@ -950,8 +949,7 @@ std::vector<std::shared_ptr<Aidge::Node>> Aidge::GraphView::getParents(const std return (it->second)->getParents(); } -std::vector<std::vector<std::shared_ptr<Aidge::Node>>> -Aidge::GraphView::getOrderedParents() const { +std::vector<std::vector<std::shared_ptr<Aidge::Node>>> Aidge::GraphView::getOrderedParents() const { std::vector<std::vector<std::shared_ptr<Node>>> parents; for (const std::shared_ptr<Node>& inputNode : inputNodes()) { parents.push_back(inputNode->getParents()); @@ -968,16 +966,14 @@ std::set<std::shared_ptr<Aidge::Node>> Aidge::GraphView::getChildren() const { return children; } -std::vector<std::vector<std::shared_ptr<Aidge::Node>>> -Aidge::GraphView::getChildren(const std::string nodeName) const { +std::vector<std::vector<std::shared_ptr<Aidge::Node>>> Aidge::GraphView::getChildren(const std::string nodeName) const { std::map<std::string, std::shared_ptr<Node>>::const_iterator it = mNodeRegistry.find(nodeName); AIDGE_ASSERT(it != mNodeRegistry.end(), "No node named {} in graph {}.", nodeName, name()); return (it->second)->getOrderedChildren(); } -std::set<std::shared_ptr<Aidge::Node>> -Aidge::GraphView::getChildren(const std::shared_ptr<Node> otherNode) const { +std::set<std::shared_ptr<Aidge::Node>> Aidge::GraphView::getChildren(const std::shared_ptr<Node> otherNode) const { std::set<std::shared_ptr<Node>>::const_iterator it = mNodes.find(otherNode); AIDGE_ASSERT(it != mNodes.end(), "The node {} (of type {}) is not in graph {}.", (otherNode) ? otherNode->name() : "#nullptr", (otherNode) ? otherNode->type() : "", name()); @@ -1088,14 +1084,10 @@ bool Aidge::GraphView::replace(const std::shared_ptr<GraphView>& oldGraph, const const std::set<NodePtr>& oldNodes = oldGraph->getNodes(); const std::set<NodePtr>& newNodes = newGraph->getNodes(); - const std::vector<std::pair<NodePtr, IOIndex_t>> oldOIn = - oldGraph->getOrderedInputs(); - const std::vector<std::pair<NodePtr, IOIndex_t>> oldOOut = - oldGraph->getOrderedOutputs(); - const std::vector<std::pair<NodePtr, IOIndex_t>> newOIn = - newGraph->getOrderedInputs(); - const std::vector<std::pair<NodePtr, IOIndex_t>> newOOut = - newGraph->getOrderedOutputs(); + const std::vector<std::pair<NodePtr, IOIndex_t>> oldOIn = oldGraph->getOrderedInputs(); + const std::vector<std::pair<NodePtr, IOIndex_t>> oldOOut = oldGraph->getOrderedOutputs(); + const std::vector<std::pair<NodePtr, IOIndex_t>> newOIn = newGraph->getOrderedInputs(); + const std::vector<std::pair<NodePtr, IOIndex_t>> newOOut = newGraph->getOrderedOutputs(); auto inputParents = std::vector<std::pair<std::shared_ptr<Node>, IOIndex_t>>(oldOIn.size()); auto outputChildren = std::vector<std::vector<std::pair<std::shared_ptr<Node>, IOIndex_t>>>(oldOOut.size()); @@ -1103,10 +1095,10 @@ bool Aidge::GraphView::replace(const std::shared_ptr<GraphView>& oldGraph, const // keep in memory every node related to the node to replace : // Parent for (std::size_t i = 0; i < oldOIn.size(); ++i) { - std::pair<NodePtr, IOIndex_t> inputParent = - oldOIn[i].first -> input(oldOIn[i].second); + std::pair<NodePtr, IOIndex_t> inputParent = oldOIn[i].first -> input(oldOIn[i].second); inputParents[i]= inputParent; // inputParent.first -> addChild(newOI[i].first, inputParent.second, newOI[i].second); + //todo Why is some code commented? I don't like this, at least explain what it's doing here } // Children for (std::size_t i = 0; i < oldOOut.size(); ++i) {