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

Improve boid flocker model and documentation #101

Merged
merged 6 commits into from
Feb 24, 2024
Merged

Conversation

coderbeta1
Copy link
Contributor

I have made the following changes:

  1. ReadMe Update: Many links in the readme were outdated due to change in folder structure. I have fixed it to work with current folders
  2. ReadMe Update: I have restructured the readme to make it similar to the readme of other examples. I have also added a link for additional information
  3. General readability: I have made general improvements by making the code more easy to read for newcomers
  4. Merge Agent and Model: I have deleted boid.py and merged it with model.py to follow best practices
  5. Renamed velocity: I have renamed the attribute "velocity" to "direction". Velocity is misleading since there is a speed attribute also and velocity is used to signify direction only in this case
  6. Model Visualization: I made it so that if there are 0 or 1 neighbours then the agent is red, else green. This help in better visual
  7. Sliders: I made it so that the user can now change the population and other parameters interactively

image

NOTE: I also tried to work on #86 for adding better visualization but I was facing some issues.

  1. As for colour I made a simple colouring scheme such that if there are less than 2 neighbours the agent is red, else green (Implemented)
  2. When I tried to make changes to the JavaScript file to add arrowHeads my changes were not being reflected. It did reflect initially, but later on it just did not reflect even the smallest change. I even deleted the entire JS file but the code still worked (it should not work without the JS file). It seemed like the python file was no longer dependent on the JavaScript file. I tried reinstalling mesa and retried but it did not work.

@rht
Copy link
Contributor

rht commented Feb 23, 2024

pre-commit.ci autofix

@@ -5,19 +5,56 @@


def boid_draw(agent):
return {"Shape": "circle", "r": 2, "Filled": "true", "Color": "Red"}
neighbors = agent.model.space.get_neighbors(agent.pos, agent.vision, False)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This operation is very expensive for all agents, for each step. At which number of agents did you notice any performance degradation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For testing I set 20 FPS. It is fine until 200 agents (which I set as upper limit). After this it slows down kinda linearly until 500, after which it is unbearably slow.

To make the colour process faster I can add the colour logic in the agent class itself. This would make it faster since we anyways need to search neighbourhood there. I can make the change as per your recommendation.

@rht
Copy link
Contributor

rht commented Feb 23, 2024

Except for the boid_draw performance measurement question, everything else LGTM.

Get the Boid's neighbors, compute the new vector, and move accordingly.
"""

neighbors = self.model.space.get_neighbors(self.pos, self.vision, False)
Copy link
Member

@EwoutH EwoutH Feb 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we make this self.neighbors we can also use if for the visualisation right?

Copy link
Contributor Author

@coderbeta1 coderbeta1 Feb 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that would work fine, but it could take too much memory with many agents. If the only need is to change colour, I think it would be more convenient to make a self.colour variable instead.

Edit: It will work but there will be a small issue. The visualization will always be one frame behind the model, this is because the neighbourhood will change after the movement but the neighbours variable will remain the same

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could save the neighbours of the previous step.

I think in many systems that compute will be more of an bottleneck than memory

Copy link
Contributor Author

@coderbeta1 coderbeta1 Feb 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rht @EwoutH I have made the changes as suggested. The model can now smoothly handle around 600 agents after which it slows down. After 1000 it is too slow and not practical

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The model can now smoothly handle around 600 agents after which it slows down. After 1000 it is too slow and not practical

That's useful info to take a note on.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First it was around 200 right? If so, that's a great improvement! Thanks!

@coderbeta1 coderbeta1 requested review from EwoutH and rht February 24, 2024 12:49
@rht
Copy link
Contributor

rht commented Feb 24, 2024

pre-commit.ci autofix

@rht rht merged commit dd5f57d into projectmesa:main Feb 24, 2024
3 checks passed
@rht
Copy link
Contributor

rht commented Feb 24, 2024

Merged, thank you.

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.

3 participants