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

LocatedAgent.interact() in need of rewriting #32

Open
roughhawkbit opened this issue Sep 2, 2013 · 2 comments
Open

LocatedAgent.interact() in need of rewriting #32

roughhawkbit opened this issue Sep 2, 2013 · 2 comments

Comments

@roughhawkbit
Copy link
Collaborator

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:

  • the head comment is out of date. This is now an interaction between one locatedAgent and its neighbours.
  • if seq is true, move() gets called twice.
  • willShove is set but never actually used. The function isMoving() simply returns ( _movement < _totalRadius/10) and is only ever called here.
  • I cannot understand the logic of ignoring shoveRadius when only shoving, but including when not only shoving. Perhaps I'm missing something but it's not commented at all.
  • I don't know what exactly if (_isAttached&!shoveOnly) {} was intended for but it no longer does anything.
@fophillips
Copy link
Contributor

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
Copy link
Collaborator Author

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

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

No branches or pull requests

2 participants