pepperplus/pepperplus-cb
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Remove SPADE dependency #29

Merged
k.marinus merged 28 commits from refactor/remove-spade into dev 2025-11-25 10:26:07 +00:00
Conversation 57 Commits 28 Files Changed 51 +2408 -3105
k.marinus commented 2025-11-21 17:28:41 +00:00 (Migrated from git.science.uu.nl)
Copy Link

Refactored the agent structure of the code base to not depend on SPADE anymore. This has lead to a massive speedup in communication speed between agents, see: https://utrechtuniversity.youtrack.cloud/articles/N25B-A-25/Speed-improvements.

To verify this merge request, first make sure that all the standard checks apply:

  • Code follows style guide
  • All tests pass
  • End-to-end functionality is intact

Since the changes are quite big, it is also necessary that all code is understandable, so that future development won't struggle trying to understand what is happening. For this, make sure the following checks apply:

  • Documentation and comments are present where needed
  • It is clear how to create a new agent with functionality, i.e.:
    • Communicating between agents
    • Adding new behavior to an agent (what was previously a CyclicBehaviour, for example)

ref: N25B-300

Refactored the agent structure of the code base to not depend on SPADE anymore. This has lead to a massive speedup in communication speed between agents, see: https://utrechtuniversity.youtrack.cloud/articles/N25B-A-25/Speed-improvements. To verify this merge request, first make sure that all the standard checks apply: * [x] Code follows style guide * [x] All tests pass * [x] End-to-end functionality is intact Since the changes are quite big, it is also necessary that all code is understandable, so that future development won't struggle trying to understand what is happening. For this, make sure the following checks apply: * [x] Documentation and comments are present where needed * [x] It is clear how to create a new agent with functionality, i.e.: * [x] Communicating between agents * [x] Adding new behavior to an agent (what was previously a `CyclicBehaviour`, for example) ref: N25B-300
k.marinus commented 2025-11-21 17:29:06 +00:00 (Migrated from git.science.uu.nl)
Copy Link

changed the description

changed the description
2584433 commented 2025-11-21 20:37:17 +00:00 (Migrated from git.science.uu.nl)
Copy Link

marked the checklist item Code follows style guide as completed

marked the checklist item **Code follows style guide** as completed
2584433 commented 2025-11-21 20:37:18 +00:00 (Migrated from git.science.uu.nl)
Copy Link

marked the checklist item All tests pass as completed

marked the checklist item **All tests pass** as completed
2584433 commented 2025-11-21 20:38:58 +00:00 (Migrated from git.science.uu.nl)
Copy Link

marked the checklist item End-to-end functionality is intact as completed

marked the checklist item **End-to-end functionality is intact** as completed
2584433 commented 2025-11-21 21:09:53 +00:00 (Migrated from git.science.uu.nl)
Copy Link

marked the checklist item It is clear how to create a new agent with functionality, i.e.: as completed

marked the checklist item **It is clear how to create a new agent with functionality, i.e.:** as completed
2584433 commented 2025-11-21 21:09:53 +00:00 (Migrated from git.science.uu.nl)
Copy Link

marked the checklist item It is clear how to create a new agent with functionality, i.e.: as incomplete

marked the checklist item **It is clear how to create a new agent with functionality, i.e.:** as incomplete
2584433 commented 2025-11-21 21:09:54 +00:00 (Migrated from git.science.uu.nl)
Copy Link

marked the checklist item Communicating between agents as completed

marked the checklist item **Communicating between agents** as completed
2584433 commented 2025-11-21 21:22:21 +00:00 (Migrated from git.science.uu.nl)
Copy Link

marked the checklist item Adding new behavior to an agent (what was previously a CyclicBehaviour, for example) as completed

marked the checklist item **Adding new behavior to an agent (what was previously a `CyclicBehaviour`, for example)** as completed
2584433 commented 2025-11-21 21:22:23 +00:00 (Migrated from git.science.uu.nl)
Copy Link

marked the checklist item It is clear how to create a new agent with functionality, i.e.: as completed

marked the checklist item **It is clear how to create a new agent with functionality, i.e.:** as completed
2584433 commented 2025-11-21 21:22:26 +00:00 (Migrated from git.science.uu.nl)
Copy Link

marked the checklist item Documentation and comments are present where needed as completed

marked the checklist item **Documentation and comments are present where needed** as completed
2584433 commented 2025-11-21 21:27:34 +00:00 (Migrated from git.science.uu.nl)
Copy Link

should Handle message maybe be an abstract method since every agent must be able to recieve messages. or the process inbox one. Just one of the two maybe?

should Handle message maybe be an abstract method since every agent must be able to recieve messages. or the process inbox one. Just one of the two maybe?
2584433 commented 2025-11-21 21:27:34 +00:00 (Migrated from git.science.uu.nl)
Copy Link

Add_background_task, docstring kinda vague, also coro type is Any.
Maybe explain a bit better how a behavoir should look like. (and refer to proper examples like the VAD or RI-Com, which both use it.

Also maybe just call it add_behavior/add_behavior_task

Add_background_task, docstring kinda vague, also coro type is Any. Maybe explain a bit better how a behavoir should look like. (and refer to proper examples like the VAD or RI-Com, which both use it. Also maybe just call it add_behavior/add_behavior_task
2584433 commented 2025-11-21 21:27:35 +00:00 (Migrated from git.science.uu.nl)
Copy Link

left review comments

left review comments
2584433 commented 2025-11-21 21:53:47 +00:00 (Migrated from git.science.uu.nl)
Copy Link

https://utrechtuniversity.youtrack.cloud/articles/N25B-A-25/Speed-improvements.

Dit document is kinda dookie ngl.
Ik zou aanraden het in een tabel te zetten. (gpt doet het zo)

Ook is de log echt pauper lang. als het over de startup speed gaat kun je het beter wat inkorten.

https://utrechtuniversity.youtrack.cloud/articles/N25B-A-25/Speed-improvements. Dit document is kinda dookie ngl. Ik zou aanraden het in een tabel te zetten. (gpt doet het zo) Ook is de log echt pauper lang. als het over de startup speed gaat kun je het beter wat inkorten.
2584433 commented 2025-11-21 21:56:50 +00:00 (Migrated from git.science.uu.nl)
Copy Link

Voor de rest ziet het er echt Minty uit tho!!
Alles werkt zoals verwacht en de tests cases zijn extensive. (had zelfs llm agent gedaan die niet eens bestond)

Voor de rest ziet het er echt Minty uit tho!! Alles werkt zoals verwacht en de tests cases zijn extensive. (had zelfs llm agent gedaan die niet eens bestond)
k.marinus commented 2025-11-22 09:04:56 +00:00 (Migrated from git.science.uu.nl)
Copy Link

I thought about making handle_message() abstract, but in fact not every agent receives messages. For example, the VADAgent doesn't. As far as _process_inbox() goes, it should not be overridden by subclasses.

I thought about making `handle_message()` abstract, but in fact not every agent receives messages. For example, the `VADAgent` doesn't. As far as `_process_inbox()` goes, it should not be overridden by subclasses.
k.marinus commented 2025-11-22 09:14:59 +00:00 (Migrated from git.science.uu.nl)
Copy Link

Ja eens, gaan we wel fixen voor het verslag

Ja eens, gaan we wel fixen voor het verslag
k.marinus commented 2025-11-22 09:28:56 +00:00 (Migrated from git.science.uu.nl)
Copy Link

changed this line in version 2 of the diff

changed this line in [version 2 of the diff](/ics/sp/2025/n25b/pepperplus-cb/-/merge_requests/29/diffs?diff_id=134074&start_sha=f8f833df640c84a5b26d919e5050702bb7a6034b#911b9fd22fa7c1eb4d856948ce37a8a49fefaae6_102_102)
k.marinus commented 2025-11-22 09:28:56 +00:00 (Migrated from git.science.uu.nl)
Copy Link

added 1 commit

Compare with previous version

added 1 commit <ul><li>1f9926fe - chore: apply suggestion</li></ul> [Compare with previous version](/ics/sp/2025/n25b/pepperplus-cb/-/merge_requests/29/diffs?diff_id=134074&start_sha=f8f833df640c84a5b26d919e5050702bb7a6034b)
8464960 commented 2025-11-22 11:00:29 +00:00 (Migrated from git.science.uu.nl)
Copy Link

requested review from @8464960

requested review from @8464960
2584433 commented 2025-11-22 12:14:10 +00:00 (Migrated from git.science.uu.nl)
Copy Link

:funcadd_background_task -> :funcadd_behavior

:func`add_background_task` -> :func`add_behavior`
8464960 commented 2025-11-22 12:31:03 +00:00 (Migrated from git.science.uu.nl)
Copy Link

Is this a configurable constant? If so, move it to the config file

Is this a configurable constant? If so, move it to the config file
8464960 commented 2025-11-22 12:31:04 +00:00 (Migrated from git.science.uu.nl)
Copy Link

same comment question about config here

same comment question about config here
8464960 commented 2025-11-22 12:31:04 +00:00 (Migrated from git.science.uu.nl)
Copy Link

Same comment/question about constants here

Same comment/question about constants here
8464960 commented 2025-11-22 12:31:04 +00:00 (Migrated from git.science.uu.nl)
Copy Link

Overall really nice job. I went over the code and I think it made the code easier to understand and implement!

I only had some very minor comments

Overall really nice job. I went over the code and I think it made the code easier to understand and implement! I only had some very minor comments
8464960 commented 2025-11-22 12:31:05 +00:00 (Migrated from git.science.uu.nl)
Copy Link

left review comments

left review comments
k.marinus commented 2025-11-22 12:37:56 +00:00 (Migrated from git.science.uu.nl)
Copy Link

Actually it shouldn't even be there, must have creeped back in with a revert

Actually it shouldn't even be there, must have creeped back in with a revert
k.marinus commented 2025-11-22 12:40:56 +00:00 (Migrated from git.science.uu.nl)
Copy Link

This is not my code so not 100% sure, but I think this is the same 1 second as one of the other constants that is already in the config file

This is not my code so not 100% sure, but I think this is the same 1 second as one of the other constants that is already in the config file
k.marinus commented 2025-11-22 12:42:08 +00:00 (Migrated from git.science.uu.nl)
Copy Link

@0950726 is this a "magic number"?

@0950726 is this a "magic number"?
0950726 commented 2025-11-22 17:41:41 +00:00 (Migrated from git.science.uu.nl)
Copy Link

Can be solved by using some sort of signal that can be awaited. Then it'll unblock immediately as well

Can be solved by using some sort of signal that can be awaited. Then it'll unblock immediately as well
0950726 commented 2025-11-22 17:45:37 +00:00 (Migrated from git.science.uu.nl)
Copy Link

Logs gaan niet om de startup speed. Het gaat om de 11 spraak detecties die tot het einde van de logs gaan. Wel kunnen sommige typen log berichten weggelaten worden. Weet sowieso niet of we de berekeningen willen reporten, uiteindelijk zijn alleen de resultaten relevant.

Logs gaan niet om de startup speed. Het gaat om de 11 spraak detecties die tot het einde van de logs gaan. Wel kunnen sommige typen log berichten weggelaten worden. Weet sowieso niet of we de berekeningen willen reporten, uiteindelijk zijn alleen de resultaten relevant.
k.marinus commented 2025-11-22 17:57:40 +00:00 (Migrated from git.science.uu.nl)
Copy Link

I feel like that would just move the loop to somewhere else, or is there an elegant way of not having to loop then?

I feel like that would just move the loop to somewhere else, or is there an elegant way of not having to loop then?
k.marinus commented 2025-11-22 18:00:23 +00:00 (Migrated from git.science.uu.nl)
Copy Link

Nevermind, it seems like an asyncio.Event can help here

Nevermind, it seems like an `asyncio.Event` can help here
k.marinus commented 2025-11-22 18:53:23 +00:00 (Migrated from git.science.uu.nl)
Copy Link

changed this line in version 3 of the diff

changed this line in [version 3 of the diff](/ics/sp/2025/n25b/pepperplus-cb/-/merge_requests/29/diffs?diff_id=134115&start_sha=1f9926fe00d6216b8cc580d580fb7c10b1b94694#717a8532402a04134a7357269a8b4ee6944ea165_49_53)
k.marinus commented 2025-11-22 18:53:24 +00:00 (Migrated from git.science.uu.nl)
Copy Link

changed this line in version 3 of the diff

changed this line in [version 3 of the diff](/ics/sp/2025/n25b/pepperplus-cb/-/merge_requests/29/diffs?diff_id=134115&start_sha=1f9926fe00d6216b8cc580d580fb7c10b1b94694#919b47b6296651ee4cab8ea0e05d681ccb11e82a_149_149)
k.marinus commented 2025-11-22 18:53:24 +00:00 (Migrated from git.science.uu.nl)
Copy Link

added 1 commit

Compare with previous version

added 1 commit <ul><li>4d076eac - perf: improved speed of BDI</li></ul> [Compare with previous version](/ics/sp/2025/n25b/pepperplus-cb/-/merge_requests/29/diffs?diff_id=134115&start_sha=1f9926fe00d6216b8cc580d580fb7c10b1b94694)
k.marinus commented 2025-11-22 18:56:15 +00:00 (Migrated from git.science.uu.nl)
Copy Link

I completely revamped this section of the BDI with the latest commit, overall performance is now much better and no more magic number :)

I completely revamped this section of the BDI with the latest commit, overall performance is now much better and no more magic number :)
k.marinus commented 2025-11-22 18:59:33 +00:00 (Migrated from git.science.uu.nl)
Copy Link

changed this line in version 4 of the diff

changed this line in [version 4 of the diff](/ics/sp/2025/n25b/pepperplus-cb/-/merge_requests/29/diffs?diff_id=134117&start_sha=4d076eac4838d8a972430207cfb2780ed59d2cbc#911b9fd22fa7c1eb4d856948ce37a8a49fefaae6_105_105)
k.marinus commented 2025-11-22 18:59:33 +00:00 (Migrated from git.science.uu.nl)
Copy Link

changed this line in version 4 of the diff

changed this line in [version 4 of the diff](/ics/sp/2025/n25b/pepperplus-cb/-/merge_requests/29/diffs?diff_id=134117&start_sha=4d076eac4838d8a972430207cfb2780ed59d2cbc#cb029a1a5b68aa61cd7e036a31c8c9dd5245e10a_119_119)
k.marinus commented 2025-11-22 18:59:33 +00:00 (Migrated from git.science.uu.nl)
Copy Link

added 1 commit

Compare with previous version

added 1 commit <ul><li>8607f9b6 - chore: apply suggestions</li></ul> [Compare with previous version](/ics/sp/2025/n25b/pepperplus-cb/-/merge_requests/29/diffs?diff_id=134117&start_sha=4d076eac4838d8a972430207cfb2780ed59d2cbc)
k.marinus commented 2025-11-22 19:11:06 +00:00 (Migrated from git.science.uu.nl)
Copy Link

added 1 commit

Compare with previous version

added 1 commit <ul><li>47a20413 - chore: fix tests</li></ul> [Compare with previous version](/ics/sp/2025/n25b/pepperplus-cb/-/merge_requests/29/diffs?diff_id=134119&start_sha=8607f9b615d331f752ae810d9f3b4d60565750d0)
k.marinus commented 2025-11-24 13:09:08 +00:00 (Migrated from git.science.uu.nl)
Copy Link

added 1 commit

  • ef00c03e - feat: pydantic models and inter-process messaging

Compare with previous version

added 1 commit <ul><li>ef00c03e - feat: pydantic models and inter-process messaging</li></ul> [Compare with previous version](/ics/sp/2025/n25b/pepperplus-cb/-/merge_requests/29/diffs?diff_id=134239&start_sha=47a20413c47e47eaa8bbe914b7c26a8964da9672)
2584433 commented 2025-11-24 16:20:02 +00:00 (Migrated from git.science.uu.nl)
Copy Link

approved this merge request

approved this merge request
0950726 commented 2025-11-24 21:10:53 +00:00 (Migrated from git.science.uu.nl)
Copy Link

approved this merge request

approved this merge request
2584433 commented 2025-11-25 08:04:09 +00:00 (Migrated from git.science.uu.nl)
Copy Link

test_agent_systems.py - test_agent_lifecycle. Faalt op het moment

test_agent_systems.py - test_agent_lifecycle. Faalt op het moment
2584433 commented 2025-11-25 08:04:09 +00:00 (Migrated from git.science.uu.nl)
Copy Link

left review comments

left review comments
k.marinus commented 2025-11-25 08:10:40 +00:00 (Migrated from git.science.uu.nl)
Copy Link

Ik heb op dit moment heel even geen tijd, maar als je in de branch docs-cb kijkt, daar is de test gefixt. Je zou dat bestand even kunnen kopieren, volgens mij zijn het twee veranderingen

Ik heb op dit moment heel even geen tijd, maar als je in de branch `docs-cb` kijkt, daar is de test gefixt. Je zou dat bestand even kunnen kopieren, volgens mij zijn het twee veranderingen
k.marinus commented 2025-11-25 08:13:58 +00:00 (Migrated from git.science.uu.nl)
Copy Link

Nevermind, dat klopt niet. Het is een race condition volgens mij, afhankelijk van hoe snel je computer is :/

Je zou het kunnen fixen door de asyncio.sleep(0.02) op regel 42 te veranderen naar 0.1 seconde, dan zou die het moeten doen. Wel een beetje rare fix natuurlijk

Nevermind, dat klopt niet. Het is een race condition volgens mij, afhankelijk van hoe snel je computer is :/ Je zou het kunnen fixen door de `asyncio.sleep(0.02)` op regel 42 te veranderen naar 0.1 seconde, dan zou die het moeten doen. Wel een beetje rare fix natuurlijk
2584433 commented 2025-11-25 08:18:18 +00:00 (Migrated from git.science.uu.nl)
Copy Link

Ja soms haalt ie hem wel en soms niet.
Met sleep op (0.1) tot nu toe altijd.
Mss idee om er straks een warning van te maken?
atm niet een groot probleem tho.

Ja soms haalt ie hem wel en soms niet. Met sleep op (0.1) tot nu toe altijd. Mss idee om er straks een warning van te maken? atm niet een groot probleem tho.
2584433 commented 2025-11-25 09:40:09 +00:00 (Migrated from git.science.uu.nl)
Copy Link

resolved all threads

resolved all threads
0950726 commented 2025-11-25 09:44:07 +00:00 (Migrated from git.science.uu.nl)
Copy Link

Kan met een:

while True:
    # This await is important. Without it, the event loop will have no opportunity to run.
    await asyncio.sleep(0.01)
    if len(agent._tasks) <= 2: break

Hierna gewoon de assert == 2 houden, dan is de functionaliteit hetzelfde.

Kan met een: ```python while True: # This await is important. Without it, the event loop will have no opportunity to run. await asyncio.sleep(0.01) if len(agent._tasks) <= 2: break ``` Hierna gewoon de `assert == 2` houden, dan is de functionaliteit hetzelfde.
k.marinus commented 2025-11-25 10:21:37 +00:00 (Migrated from git.science.uu.nl)
Copy Link

added 1 commit

Compare with previous version

added 1 commit <ul><li>e5949a72 - fix: fix test race condition</li></ul> [Compare with previous version](/ics/sp/2025/n25b/pepperplus-cb/-/merge_requests/29/diffs?diff_id=134323&start_sha=ef00c03ec596294a245d575e510142db8f5eb5ad)
k.marinus commented 2025-11-25 10:24:34 +00:00 (Migrated from git.science.uu.nl)
Copy Link

It has been fixed!

It has been fixed!
k.marinus commented 2025-11-25 10:24:37 +00:00 (Migrated from git.science.uu.nl)
Copy Link

resolved all threads

resolved all threads
k.marinus commented 2025-11-25 10:26:07 +00:00 (Migrated from git.science.uu.nl)
Copy Link

mentioned in commit 435f0c25a9

mentioned in commit 435f0c25a9b1c57b2dca394880b22852e8adeecd
k.marinus (Migrated from git.science.uu.nl) merged commit 435f0c25a9 into dev 2025-11-25 10:26:07 +00:00
2584433 (Migrated from git.science.uu.nl) approved these changes 2026-02-02 13:29:06 +00:00
0950726 (Migrated from git.science.uu.nl) approved these changes 2026-02-02 13:29:06 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
2584433
0950726
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
kasper twirre pepperplus
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: pepperplus/pepperplus-cb#29
Delete Branch "refactor/remove-spade"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?