kreft / iDynoMiCS

Individual-based Dynamics of Microbial Communities Simulator
Other
18 stars 37 forks source link

LocatedAgent.interact() in need of rewriting #32

Open roughhawkbit opened 11 years ago

roughhawkbit commented 11 years ago

As it currently stands:

/**
     * \brief Models a mechanical interaction between two located agents. Implemented by extending classes (LocatedAgent)
     * 
     * Models a mechanical interaction between two located agents. Implemented by extending classes (LocatedAgent)
     * 
     * @param MUTUAL    Whether movement is shared between two agents or applied only to this one
     * @param shoveOnly Boolean noting whether this action is shoving (false) or pulling (shrinking biofilm) (true)
     * @param seq   Whether the move should be applied immediately or wait until the end of the step
     * @param gain  Double noting change in position
     * @return  The move to be applied once the shoving or pull calculations have been performed
     */
    public double interact(boolean MUTUAL, boolean shoveOnly, boolean seq,
            double gain) {
        boolean willShove = false;

        move();

        // rebuild your neighbourhood
        if (shoveOnly)
            getPotentialShovers(getInteractDistance());
        else
            getPotentialShovers(getInteractDistance() + getShoveRadius());

        Iterator<LocatedAgent> iter = _myNeighbors.iterator();
        while (iter.hasNext()) {
            if (shoveOnly)
                willShove |= addPushMovement(iter.next(), MUTUAL, gain);
            else
                willShove |= addSpringMovement(iter.next(), MUTUAL, gain);

        }
        _myNeighbors.clear();

        // Check interaction with surface
        if (_isAttached&!shoveOnly) {

        }

        willShove = isMoving();

        if (seq)
            return move();
        else
            return 0;
    }

Issues:

fophillips commented 10 years ago

As far as I can tell, the reason move() is called at the beginning of the method is to perform movement due to the current agent of interest being a neighbour of any other agents in previous iterations of the loop in which interact is called (ie it's _movement vector may be non-zero at the start of the method), this will ensure the potential shovers found are for the new location. Then the second move() call will be for movement due to the new neighbours.

However this leads to a potential bug if seq is false and the desired behaviour is keep the agents stationary until the end of the step, and the first move() call will counter this.

But this is currently avoided in the code by hardcoding seq=true in the only place interact is called on line 590 of AgentContainer.java:

performMove(shoveOnly, false, 1);

where the false is not-ted when interact is called.

Issue #36 may be related to this.

roughhawkbit commented 10 years ago

It's worth pointing out that AgentContainer.performMove(pushOnly, isSynchro, gain) is only ever called by AgentContainer.shoveAllLocated(fullRelax, shoveOnly, maxShoveIter, gainMin, gainMax), which always sets isSynchro to false (line 590). This means that seq in LocatedAgent.interact(MUTUAL, shoveOnly, seq, gain) is always true, and so move() is always called twice