davidrmiller / biosim4

Biological evolution simulator
Other
3.1k stars 435 forks source link

Added kill sensory neuron (#2) #12

Closed bolshoytoster closed 1 year ago

bolshoytoster commented 2 years ago

Added two new sensors, KILLS & SELF_KILLS. KILLS returns the total number of kills of individuals in the area divided by the total kills. SELF_KILLS returns the current individual's kills divided by the total kills.

These are both disabled by default, because the KILL_FORWARD action is and you could probably get rid of SELF_KILLS since it's not as useful.

I've also made the murderCount variable in simulator.cpp global so it can be accessed in getSensor.cpp.

davidrmiller commented 2 years ago

@bolshoytoster , this is an interesting enhancement and will be fun to experiment with. I'd like to look more closely at this after we get some of the code cleaned up and think about testing methodology, then we can think more long-term about expanding sensors and actions. Thanks for the work you did on this.

davidrmiller commented 2 years ago

I've been playing around with this PR and I like the concept of the new kill sensors, but there are details that need further thought.

This PR relies on the global variable murderCount, but murderCount is only an approximation. It is incremented at the end of each sim step by the size of the deferred death queue. A single individual can be queued for death multiple times if it is killed by multiple neighbors during the same sim step. An individual can also be queued for death if it dies by non-violent means such as when the "radioactive" scenario is enabled. The size of the death queue therefore is equal to or greater than the number of unique individuals actually deceased.

This PR introduces the new member variable Indiv::kills to count the number of murders committed by each individual. It is incremented in executeActions.cpp when an individual causes another individual to be queued for death. However, since the same individual can be queued for death by multiple murderers, multiple killers get charged for the same kill. Perhaps that's ok -- if A and B both kill C, it's reasonable that A and B are equally culpable.

The result is that Indiv::kills and the global murderCount are only approximations. In getSensor.cpp, handlers for the new KILLS and SELF_KILLS sensors compute a sensor value from these approximations. Maybe that's ok if we counted violent and non-violent deaths separately.

Here's an example scenario: suppose C dies three times in a sim step where A and B both kill C, and C is additionally killed by some non-violent means. That will cause C to be put on the death queue three times, but only two individuals are murderers, and only one murder should be recorded in logs/epoch-log.txt.

bolshoytoster commented 2 years ago

This could be resolved if murderCount is instead incremented when the indivs are killed, in Peeps::drainDeathQueue:

https://github.com/davidrmiller/biosim4/blob/2310473213783df3c14ff7d7d14d7da6774fc8b5/src/peeps.cpp#L42-L52

This could be changed to:

void Peeps::drainDeathQueue() 
{
    for (uint16_t index : deathQueue) {
        if (indiv.alive) {
            auto & indiv = peeps[index];
            grid.set(indiv.loc, 0);
            indiv.alive = false;
            ++murderCount
        }
    }
    deathQueue.clear();
}

Then obviously remove murderCount += peeps.deathQueueSize(); from line 146 of simulation.cpp


Regarding environmental deaths, if we want to not count them we’d have to do something different. In executeActions - at Action::KILL_FORWARD: https://github.com/davidrmiller/biosim4/blob/2310473213783df3c14ff7d7d14d7da6774fc8b5/src/executeActions.cpp#L125-L141

Just before peeps.queueForDeath(indiv2) we’d have to make sure indiv2.index isn’t in deathQueue. A quick stackoverflow search says it won’t be hard - but it would probably be inefficient to search the whole queue each time an indiv is killed, so I’d just leave it as the above suggestion.

petterreinholdtsen commented 2 years ago

Any hope to have this branch rebased on the latest main to make it easier to test?

bolshoytoster commented 2 years ago

@petterreinholdtsen I'll do that now.

petterreinholdtsen commented 1 year ago

@bolshoytoster Did you give up getting this change into the main branch, or was there some other reason you closed this pull request?

bolshoytoster commented 1 year ago

@petterreinholdtsen I gave up, since there was no activity on the PR for over a year.

petterreinholdtsen commented 1 year ago

[bolshoytoster]

@petterreinholdtsen I gave up, since there was no activity on the PR for over a year.

I do not blame you.

I've had bugs I reported fixed and patches I provied applied more than 10 years later, and while I did give up in the mean time, I was very happy when the fix finally was published for everyone to use. I am glad I left the patch and bug report open even though I gave up, as someone new might take over some time in the future to find the issue.

-- Happy hacking Petter Reinholdtsen