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

Why are we storing the current Node the agent is on as a dictionary? #47

Closed
DanielPerezJensen opened this issue May 28, 2020 · 5 comments · Fixed by #49
Closed

Why are we storing the current Node the agent is on as a dictionary? #47

DanielPerezJensen opened this issue May 28, 2020 · 5 comments · Fixed by #49
Assignees
Labels
bug Something isn't working

Comments

@DanielPerezJensen
Copy link
Owner

In graph.py we are storing self.current like this:

self.current = {agent_id: self.nodes[(0, 0)]}

Why are we storing our current node like this instead of as a coordinate? Even though in the rest of the functions we always use the location of nodes instead of the specific instance of a node? Can we atleast add a variable that tracks the current location as a pair of coordinates?

@DanielPerezJensen DanielPerezJensen added the bug Something isn't working label May 28, 2020
@dorian4840
Copy link
Collaborator

There isn't a specific reason why we store the Node object instead of the coordinates.

We noticed that the agents in the graph need to store more than just their current location (energy and attached blocks, for example), so we will change self.current = {agent_id: self.nodes[(0, 0)]} to something like self.info {agent_id: {'node': self.nodes[(0, 0)], 'energy': 300}}. Then we could make separate keys for the node object and the location. It's just that by simple doing Node.location you can get the location of the node, so storing the location as well just seems like an unnecessary thing to keep track of.

We could only store the location and not the Node Object itself, but then we would have to change the entire code for something that doesn't add extra clarify or practical value

@DanielPerezJensen
Copy link
Owner Author

That makes sense, but the reason I find it a bit unwieldy is this, I want to check if a given node is the node an agent is currently located in. Because of the dictionary structure I hvae to do it like this:

if node.location == self.beliefs.current[self._user_id].location

It would be nicer if we could have either:

  • A function that does it for us
if node.location == self.current_location()

or

  • Simply storing it as a named variable in the Graph object
if node.location == self.current_location

@dorian4840
Copy link
Collaborator

You're right, it is a bit of a mouthful. I'll create the current_location() function (if we were to do a variable we would also need to keep updating it).

@dorian4840
Copy link
Collaborator

I added the function current_info() to agent.py.

def current_info(self, info):
    if info == 'location':
        return self.beliefs.get_current(self._user_id).location
    elif info == 'node':
        return self.beliefs.get_current(self._user_id)

This way you can easily get the current location as well as the node, + you can easily also add other information in necessary

dorian4840 added a commit that referenced this issue May 29, 2020
Added current_info() in agent.py to solve issue 47.
Improved get_node() in graph.py to solve issue 48.

[Ticket: #47]
[Ticket: #48]
@DanielPerezJensen
Copy link
Owner Author

I would split it up as two functions, current_location() and current_node(). I'll do it in a separate branch.

DanielPerezJensen added a commit that referenced this issue May 29, 2020
Closes #47 by making a function that returns the node in which the
agent is currently located and a function that returns the location
on which the agent is currently located. current_node() and
current_location() respectively.

[Ticket: #47]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants