-
Notifications
You must be signed in to change notification settings - Fork 172
virus antibody model #253
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
base: main
Are you sure you want to change the base?
virus antibody model #253
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request introduces a simulation that models the interaction of viruses and antibodies in continuous space using the Mesa framework. The PR implements the core model logic, defines agent behaviors with dual memory systems and weak referencing, and sets up visualization components with an accompanying README for usage instructions.
- Implements the core Virus/Antibody simulation with model parameters and agent scheduling.
- Adds visualization components and parameter interfaces in the application module.
- Documents the model details, simulation outcomes, and usage instructions in the README.
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
examples/virus_antibody/model.py | Implements the primary model setup and simulation step behavior. |
examples/virus_antibody/app.py | Provides the visualization interface and model parameter configurations. |
examples/virus_antibody/agents.py | Defines antibody and virus agent behaviors with memory, duplication, and engagement logic. |
examples/virus_antibody/README.md | Provides documentation covering model overview, usage, and simulation examples. |
Files not reviewed (1)
- examples/virus_antibody/requirements.txt: Language not supported
) | ||
|
||
# Create and place the Antibody agents | ||
antibodies_positions = self.rng.random( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent use of random number generators (self.rng vs self.random) may lead to unpredictable behavior; consider unifying these to a single random instance.
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
examples/virus_antibody/agents.py
Outdated
|
||
# Movement & state | ||
self.position = initial_position | ||
self.speed = 1.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to make speed and health kwargs on the init? In my view it is good practice to make all attributes explicitly controllable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made the choice of limiting the parameter to these ones because otherwise it's a lot for the user in my opinion. If I add all of them, it would mean more than 10 parameters to be decided by the user. Do you think I should still add all of the other parameters ? Or are there one or two more that would be good to add and still leave the others hidden (like ko_timeout, sight_range, and a few non-crucial parameters) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. One option is to turn the constants that are the same for all agents of a given class into class level attributes. This still makes it easy to change them without having to add them all to the signature of the __init__
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea ! It does allow to delete quite a few lines of code in model.py
also.
|
||
# Chase a valid virus target | ||
else: | ||
if getattr(target, "space", None) is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again, this should not be needed if everything else is implemented correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have made a first pass. I like the idea of the model, but there is ample room for improvements. If you have questions, don't hesitate to ask.
self.lt_memory.append(dna) | ||
self.ko_steps_left = self.ko_timeout | ||
# mark KO state by weak-ref back to self | ||
self.target = weakref.ref(self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont understand what is going on here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an antibody looses a confrontation with a virus (because the virus DNA is not in it's memory yet), the antibody is ko for a few steps. It can't move during this time and I symbolise this by setting the agent's target to itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, clear.
This might be implemented using event scheduling, which would result in a modest speedup of runtime.
Thanks a lot @quaquel for this first review ! I'm going to see what I can change asap |
I would start with the two sets in the the init of your model. Next, I would remove the if checks you have to see if an agent is still active in the model one at a time (or do one first to confirm the behavior is still correct). Again, don't hesitate to ask if you run into any issues. |
for more information, see https://pre-commit.ci
@quaquel I removed the checks and did the other fixes that you suggested. I still have one error that I don't understand, and it seems to be still linked to referencing. For certain configurations of parameters, the model runs for a while (even hundreds of steps) before making the error You can reproduce this error by keeping the base parameters and setting ![]() Full traceback :
|
I'll try to take a closer look asap. It indeed seems that step is called on an agent that has already been removed. |
examples/virus_antibody/agents.py
Outdated
return dna | ||
|
||
def move(self): | ||
# Random walk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both agents have a random walk component. Might it be possible to implement this the same way for both and move it to a new super agent class from which both current agents derive?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be a good idea ! At first I thought it could also be used for the duplicate()
method. However, there are a lot of parameters to take into account and the code is very short inside, so I don't think that it would really help to make it derive from a parent class.
So I the end, there is just a few lines for the random movement. Isn't a whole parent class a bit overkill ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the choice is between repeated code or a parent class, I prefer the parent class. Of course, in python you could also move the relevant code into a helper function (not really appropriate for random walks).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, will do, thank you for the advice !
On another note, did you have time to check out why step was being called on a removed agent ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Monday was a banking holiday so I had family obligations. I'll try to get to it today, but no promises unfortunately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, nothing urgent ! Only if you have time :)
Virus-Antibody Model
This model is a simulation of immune reaction declined as a confrontation between antibody agents and virus agents. The global idea is to model how the imune system can struggle against new virus but is able to adapt over time and beat a same virus if it comes back. The results are quite interesting as the simulation can go both ways (virus win or antibodies win) with a little tweak in the base parameters (check the readme for some examples).
What it showcases
self.target
attribute)What it adds to mesa-examples
How It Works