[DF] Various improvements in SaveGraph #9145

eguiraud · 2021-10-20T08:25:32Z

SaveGraph would benefit from the following improvements:

usage of static variables should be removed, in order to make SaveGraph safe to be called concurrently from multiple threads
a tutorial should be added, and an example usage should be included in the docs in the reference guide

Additional context

Related old jira tickets:

https://sft.its.cern.ch/jira/browse/ROOT-11023 (this is the original issue regarding the usage of statics in the implementation of SaveGraph)
https://sft.its.cern.ch/jira/browse/ROOT-9721 (this contains a draft tutorial and an old SaveGraph bug report)

The text was updated successfully, but these errors were encountered:

SaveGraph mainly relied on static structures. Removed static maps, which were used to check if a define/filter/range node were already on the computation graph. Solution is to pass a (non-static) map, which is created at each call of SaveGraph. Changes do not affect existing test cases.

…roject#9145) SaveGraph now does not need a static method to update the id of the nodes. There is no resetting of the id counter to 0 now. Corresponding tests are updated.

…#9145)

…roject#9145) SaveGraph now does not need a static method to update the id of the nodes. There is no resetting of the id counter to 0 now. Corresponding tests are updated.

…oot-project#9145)

SaveGraph mainly relied on static structures. Removed static maps, which were used to check if a define/filter/range node were already on the computation graph. Solution is to pass a (non-static) map, which is created at each call of SaveGraph. SaveGraph now does not need a static method to update the id of the nodes. There is no resetting of the id counter to 0 now. Corresponding tests are updated. A python tutorial is included.

SaveGraph mainly relied on static structures. Removed static maps, which were used to check if a define/filter/range node were already on the computation graph. Solution is to pass a (non-static) map, which is created at each call of SaveGraph. SaveGraph now does not need a static method to update the id of the nodes. There is no resetting of the id counter to 0 now. Corresponding tests are updated. A python tutorial is included. Added missing files

…ect#9145)

SaveGraph mainly relied on static structures. Removed static maps, which were used to check if a define/filter/range node were already on the computation graph. Solution is to pass a (non-static) map, which is created at each call of SaveGraph. SaveGraph now does not need a static method to update the id of the nodes. There is no resetting of the id counter to 0 now. Corresponding tests are updated. A python tutorial is included.

SaveGraph mainly relied on static structures. Removed static maps, which were used to check if a define/filter/range node were already on the computation graph. Solution is to pass a (non-static) map, which is created at each call of SaveGraph. Get rid of the static id initializer. The size of the map of visited nodes is used to assign unique ids. Now, also the action nodes are in the visited map. The visited map is now only one and has signature std::unordered_map<void *, std::shared_ptr<GraphNode>> - in this manner, different type of nodes can use the same map. Tests were adapted accordingly.

…ect#9145)

SaveGraph mainly relied on static structures. Removed static maps, which were used to check if a define/filter/range node were already on the computation graph. Solution is to pass a (non-static) map, which is created at each call of SaveGraph. Get rid of the static id initializer. The size of the map of visited nodes is used to assign unique ids. Now, also the action nodes are in the visited map. The visited map is now only one and has signature std::unordered_map<void *, std::shared_ptr<GraphNode>> - in this manner, different type of nodes can use the same map. Tests were adapted accordingly.

…ot-project#9145) SaveGraph mainly relied on static structures. Removed static maps, which were used to check if a define/filter/range node were already on the computation graph. Solution is to pass a (non-static) map, which is created at each call of SaveGraph. Get rid of the static id initializer. The size of the map of visited nodes is used to assign unique ids. Now, also the action nodes are in the visited map. The visited map is now only one and has signature std::unordered_map<void *, std::shared_ptr<GraphNode>> - in this manner, different type of nodes can use the same map. Moreover, some friend methods are now redundant. Tests were adapted accordingly.

…ot-project#9145) SaveGraph mainly relied on static structures. Removed static maps, which were used to check if a define/filter/range node were already on the computation graph. Solution is to pass a (non-static) map, which is created at each call of SaveGraph. Get rid of the static id initializer. The size of the map of visited nodes is used to assign unique ids. Now, also the action nodes are in the visited map. The visited map is now only one and has signature std::unordered_map<void *, std::shared_ptr<GraphNode>> - in this manner, different type of nodes can use the same map. Moreover, some friend methods are now redundant. Tests were adapted accordingly. [DF] Fixed SaveGraph Tutorials

…ot-project#9145) Removed the static GraphNode id initializer (relevant for SaveGraph). Removed static maps, which were used to check if a define/filter/range node were already on the computation graph. Solution is to pass a (non-static) map, which is created at each call of SaveGraph. The visited map is now only one and has signature std::unordered_map<void *, std::shared_ptr<GraphNode>> - in this manner, different type of nodes can use the same map. The size of the map of visited nodes is used to assign unique ids. Now, also the action nodes are in the visited map. Moreover, some friend methods are now redundant. Tests were adapted accordingly.

…ot-project#9145) Removed the static GraphNode id initializer (relevant for SaveGraph). Removed static maps, which were used to check if a define/filter/range node were already on the computation graph. Solution is to pass a (non-static) map, which is created at each call of SaveGraph. The visited map is now only one and has signature std::unordered_map<void *, std::shared_ptr<GraphNode>> - in this manner, different type of nodes can use the same map. The size of the map of visited nodes is used to assign unique ids. Now, also the actions and the RLoopManager are in the visited map. Moreover, some friend methods are now redundant. Tests were adapted accordingly.

) Removed the static GraphNode id initializer (relevant for SaveGraph). Removed static maps, which were used to check if a define/filter/range node were already on the computation graph. Solution is to pass a (non-static) map, which is created at each call of SaveGraph. The visited map is now only one and has signature std::unordered_map<void *, std::shared_ptr<GraphNode>> - in this manner, different type of nodes can use the same map. The size of the map of visited nodes is used to assign unique ids. Now, also the actions and the RLoopManager are in the visited map. Moreover, some friend methods are now redundant. Tests were adapted accordingly.

…ot-project#9145) Removed the static GraphNode id initializer (relevant for SaveGraph). Removed static maps, which were used to check if a define/filter/range node were already on the computation graph. Solution is to pass a (non-static) map, which is created at each call of SaveGraph. The visited map is now only one and has signature std::unordered_map<void *, std::shared_ptr<GraphNode>> - in this manner, different type of nodes can use the same map. The size of the map of visited nodes is used to assign unique ids. Now, also the actions and the RLoopManager are in the visited map. Moreover, some friend methods are now redundant. Tests were adapted accordingly.

eguiraud added improvement in:RDataFrame labels Oct 20, 2021

eguiraud assigned ikabadzhov Oct 20, 2021

eguiraud added the priority:low label Oct 20, 2021

ikabadzhov mentioned this issue Jan 6, 2022

Remove static members from SaveGraph (fix #9145) #9503

Closed

2 tasks

ikabadzhov added a commit to ikabadzhov/root that referenced this issue Jan 7, 2022

fixup! Remove static maps from SaveGraph (partial fix of root-project…

26f63bf

…#9145)

ikabadzhov added a commit to ikabadzhov/root that referenced this issue Jan 7, 2022

fixup! Remove static method to keep unique node ids in SaveGraph (fix r…

61b9544

…oot-project#9145)

ikabadzhov added a commit to ikabadzhov/root that referenced this issue Jan 7, 2022

fixup! Remove static members from SaveGraph (partial fix of root-proj…

c5a5075

…ect#9145)

ikabadzhov added a commit to ikabadzhov/root that referenced this issue Jan 10, 2022

fixup! Remove static members from SaveGraph (partial fix of root-proj…

cbeb381

…ect#9145)

ikabadzhov mentioned this issue Jan 11, 2022

[DF] Remove SaveGraph static members; Add SaveGraph tutorials #9540

Merged

2 tasks

eguiraud closed this as completed in #9540 Jan 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DF] Various improvements in SaveGraph #9145

[DF] Various improvements in SaveGraph #9145

eguiraud commented Oct 20, 2021 •

edited

Loading

[DF] Various improvements in SaveGraph #9145

[DF] Various improvements in SaveGraph #9145

Comments

eguiraud commented Oct 20, 2021 • edited Loading

Additional context

eguiraud commented Oct 20, 2021 •

edited

Loading