Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(WIP) Attempt to eliminate cyclic dependency NAR→Cycle→Decision #280

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ARCJ137442
Copy link
Contributor

Maybe reducing cyclic dependency can make the module dependency of ONA better?

  • Privatize the global variable currentTime in NAR.c - completely possible
  • Supplement the parameter currentTime of Decision_ConsiderNegativeOutcomes to eliminate the cyclic dependency of NAR -> Cycle -> Decision - still WIP and needs further discussions

The only problem is here:

NAR_AddInputBelief(feedbackTerm);

Are there any good alternatives to "input beliefs"?
Pass a function pointer such like void (*InputBelief) (Term) as a parameter on Decision_Execute, Cycle_ProcessAndInferGoalEvents and Cycle_Perform?

…urrentTime in `NAR.c` and supplement the parameter `currentTime` of `Decision_ConsiderNegativeOutcomes` to eliminate the cyclic dependency of `NAR` -> `Cycle` -> `Decision`
… `currentTime` in `NAR.c` and resolves compile-time errors
The global variable `currentTime` is now completely private for NAR in `NAR.c`
@ARCJ137442 ARCJ137442 changed the title Attempt to eliminate cyclic dependency NAR→Cycle→Decision (WIP) Attempt to eliminate cyclic dependency NAR→Cycle→Decision Sep 22, 2024
@patham9
Copy link
Member

patham9 commented Sep 25, 2024

Awesome that you found this, thank you!
And I will think about a solution to replacing NAR_AddInputBelief(feedbackTerm) there.

@ARCJ137442
Copy link
Contributor Author

Awesome that you found this, thank you! And I will think about a solution to replacing NAR_AddInputBelief(feedbackTerm) there.

Thanks for your watching too!

I also have some extra information that I hope will help us.

That's my dependancy analyze of ONA using Graphviz dot:

ONA include graph

(generated on 2024-09-21, .svg vectorgraph)

And its Graphviz code can be seen and previewed here.
(2 cyclic dependencies: Narsese.hVariable.h, Decision.hNAR.hCycle.h)

Perhaps it can give us a macro overview of module dependencies in ONA :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants