dlr-gtlab / intelligraph-module

A Node-based Workflow Engine for GTlab
1 stars 0 forks source link

Resolve "Refactor the graph execution model to properly support nested graphs" #189

Closed rainman110 closed 1 month ago

rainman110 commented 1 month ago

In GitLab by @mariusalexander on Sep 12, 2024, 08:51

Closes #112

Closes #34

Finally, after a quite long cooking session the exec model overhaul is here. It should be stable and handle much more as expected in most if not all cases.

The biggest change is that the exec model operates on the "global connection model", which spans over the whole hierarchy of a graph (i.e. includes nested graphs). As a result, the exec model uses NodeUuids instead of NodeIds which may cause a few inconveniences when using the API (we may have to look into switching entirely to NodeUuids at some point). The old model operated on a single level only, which caused a few issues and inconsistencies across nested graphs. While the overall implementation is a bit lengthier than expected, I hope it is a bit more concise. Still there a few things I would like to touch up in the future.

In addition the API has been cleaned up. This is especially true for the future-object, which allows the registration of callback functions and "joining" of two future objects into one future.

Lastly, the coupling between all components has further been decreased. In the last major PR (refactoring of the connection model) I could decouple the Node and NodeGraphicsObject class from the graph class. In this PR I could eliminate the coupling between most components and the exec model, except the view, scene and node-ui class. Still, there might be a few more things to do on this front to allow easy swapping of exec models in the future.

rainman110 commented 1 month ago

In GitLab by @mariusalexander on Sep 12, 2024, 17:51

added 3 commits

Compare with previous version

rainman110 commented 1 month ago

In GitLab by @mariusalexander on Sep 13, 2024, 08:40

added 1 commit

Compare with previous version

rainman110 commented 1 month ago

In GitLab by @mariusalexander on Sep 16, 2024, 09:27

added 3 commits

Compare with previous version

rainman110 commented 1 month ago

In GitLab by @mariusalexander on Sep 20, 2024, 16:58

added 1 commit

Compare with previous version

rainman110 commented 1 month ago

In GitLab by @mariusalexander on Sep 20, 2024, 18:53

added 8 commits

Compare with previous version

rainman110 commented 1 month ago

In GitLab by @mariusalexander on Sep 20, 2024, 18:55

changed target branch from main to update_lincensing

rainman110 commented 1 month ago

In GitLab by @mariusalexander on Sep 20, 2024, 18:56

changed target branch from update_lincensing to main

mariusalexander commented 1 month ago

@rainman110 I think the last two cppcheck warnings are bugged and I cannot get rid of them using cppcheck-suppress-statements. Is there any other way to silence these (incorrect) warnings? The PR is then finished.

rainman110 commented 1 month ago

@rainman110 I think the last two cppcheck warnings are bugged and I cannot get rid of them using cppcheck-suppress-statements. Is there any other way to silence these (incorrect) warnings? The PR is then finished.

I think this should be // cppcheck-suppress constParameter, see here for example:

https://github.com/dlr-gtlab/python-module/blob/43f2e2cd41354425c04cc41dfa3ccb543011e05a/src/module/utilities/gtpypp.h#L174C1-L178

mariusalexander commented 1 month ago

I think this should be // cppcheck-suppress constParameter,

Oh I see, a rookie mistake... :joy: Thanks!