google-deepmind / concordia

A library for generative social simulation
Apache License 2.0
633 stars 148 forks source link

[BUG] Access to RecentMemories in agent_components_tutorial.ipynb #86

Closed SoyGema closed 3 weeks ago

SoyGema commented 3 weeks ago

Hello there, Thanks for making the tutorials ! They are very didactic .My apologies in advance for the inconvenience In the agent_components_tutorial.ipnynb , while reproducting the step in which a memory component is introduced , bump into the following error when executing the notebook.

Captura de pantalla 2024-09-25 a las 15 01 02

It seems that the object is not subscriptable so I inspected quickly the memory attributes and this was printed

{'text': 'You are in a room.', 'score': 0.0}
{'text': 'The room has only a table in it.', 'score': 0.0}
{'text': 'On the table there are two fruits: an apple and a banana.', 'score': 0.0}
{'text': 'The apple is shinny red and looks absolutely irresistible!', 'score': 0.0}
{'text': 'The banana is slightly past its prime.', 'score': 0.0}

Thereforre , I changed

recent_memories = " ".join(memory[0] for memory in recent_memories_list) for

recent_memories = " ".join(memory.text for memory in recent_memories_list) in the following code snippet and it worked fine

class RecentMemories(entity_component.ContextComponent):

  def pre_act(self, action_spec) -> None:
    recent_memories_list = self.get_entity().get_component('memory').retrieve(
        query='',  # Don't need a query to retrieve recent memories.
        limit=5,
        scoring_fn=legacy_associative_memory.RetrieveRecent(),
    )

    recent_memories = " ".join(memory[0] for memory in recent_memories_list)
    print(f"*****\nDEBUG: Recent memories:\n  {recent_memories}\n*****")
    return recent_memories

It seems it goes to utils/concurrency.py so I donยดt want to assume that the access here is misguided...shall canonically be indexed as list element or calling the attribute? Apologies if Iยดm misunderstanding something. MemoryComponent design fundamentals is important for me to get it right, as it could influence component designs such as promise-keeping or other time-dependent stored aspects. I guess I will find out as I will dig in but also wondering why MemoryComponent was not used and ContextComponent is in this case. Fascinating framework Please let me know any way I could contribute to fix this and my apologies for the inconvenience again ๐Ÿ™๐Ÿ™๐Ÿ™ Have a nice day ๐Ÿ™๐Ÿ™๐Ÿ™

vezhnick commented 3 weeks ago

Thank you so much for finding yet another bug! We are submitting an update shortly that fixes it.