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

Removing agentControllerType initialization parameter on nanna branch. #1048

Merged
merged 3 commits into from
Jul 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ai2thor/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,12 @@ def __init__(
DeprecationWarning,
)

if "agentControllerType" in self.initialization_parameters:
raise ValueError(
"`agentControllerType` is no longer an allowed initialization parameter."
" Use `agentMode` instead."
)

# Let's set the scene for them!
if scene is None:
scenes_in_build = self.scenes_in_build
Expand Down
50 changes: 24 additions & 26 deletions ai2thor/tests/test_unity.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,24 @@
# import pytest
import copy
import glob
import json
import os
import string
import random
import re
import copy
import json
import string
import time

import pytest
import warnings

import jsonschema
import numpy as np
import pytest
from PIL import ImageChops, ImageFilter, Image

from ai2thor.controller import Controller
from ai2thor.fifo_server import FifoServer
from ai2thor.tests.constants import TESTS_DATA_DIR, TEST_SCENE
from ai2thor.wsgi_server import WsgiServer
from ai2thor.fifo_server import FifoServer
from PIL import ImageChops, ImageFilter, Image
import glob


# Defining const classes to lessen the possibility of a misspelled key
class Actions:
Expand Down Expand Up @@ -59,7 +61,6 @@ def build_controller(**args):

_wsgi_controller = build_controller(server_class=WsgiServer)
_fifo_controller = build_controller(server_class=FifoServer)
_stochastic_controller = build_controller(agentControllerType="stochastic", agentMode="stochastic")


def skip_reset(controller):
Expand All @@ -86,10 +87,6 @@ def wsgi_controller():
return reset_controller(_wsgi_controller)


@pytest.fixture
def stochastic_controller():
return reset_controller(_stochastic_controller)


@pytest.fixture
def fifo_controller():
Expand All @@ -98,7 +95,6 @@ def fifo_controller():

fifo_wsgi = [_fifo_controller, _wsgi_controller]
fifo = [_fifo_controller]
fifo_wsgi_stoch = [_fifo_controller, _wsgi_controller, _stochastic_controller]

BASE_FP28_POSITION = dict(x=-1.5, z=-1.5, y=0.901,)
BASE_FP28_LOCATION = dict(
Expand All @@ -114,12 +110,12 @@ def teleport_to_base_location(controller: Controller):


def setup_function(function):
for c in fifo_wsgi_stoch:
for c in fifo_wsgi:
reset_controller(c)


def teardown_module(module):
for c in fifo_wsgi_stoch:
for c in fifo_wsgi:
c.stop()


Expand All @@ -139,16 +135,18 @@ def assert_images_far(image1, image2, min_mean_pixel_diff=10):
return np.mean(np.abs(image1 - image2).flatten()) >= min_mean_pixel_diff


def test_stochastic_controller(stochastic_controller):
stochastic_controller.reset(TEST_SCENE)
assert stochastic_controller.last_event.metadata["lastActionSuccess"]
def test_agent_controller_type_no_longer_accepted(fifo_controller):
with pytest.raises(ValueError):
build_controller(
server_class=FifoServer,
agentControllerType="physics",
agentMode="default",
)

def test_stochastic_mismatch(fifo_controller):
try:
c = fifo_controller.reset(agentControllerType="stochastic", agentMode="default")
except RuntimeError as e:
error_message = str(e)
assert error_message and error_message.startswith("Invalid combination of agentControllerType=stochastic and agentMode=default")
# TODO: We should make ServerAction type actions fail when passed
# invalid arguments.
# with pytest.raises(Exception):
# fifo_controller.reset(agentControllerType="physics", agentMode="default")


# Issue #514 found that the thirdPartyCamera image code was causing multi-agents to end
Expand Down Expand Up @@ -340,7 +338,7 @@ def test_target_invocation_exception(controller):
], "errorMessage should not be empty when OpenObject(x > 1)."


@pytest.mark.parametrize("controller", fifo_wsgi_stoch)
@pytest.mark.parametrize("controller", fifo_wsgi)
def test_lookup(controller):

e = controller.step(dict(action="RotateLook", rotation=0, horizon=0))
Expand Down
95 changes: 21 additions & 74 deletions unity/Assets/Scripts/AgentManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -167,90 +167,38 @@ public void Initialize(ServerAction action) {
//"locobot" agentMode can use either default or "stochastic" agentControllerType
//"drone" agentMode can ONLY use "drone" agentControllerType, and NOTHING ELSE (for now?)
if (action.agentMode.ToLower() == "default") {
if (action.agentControllerType.ToLower() != "physics" && action.agentControllerType.ToLower() != "stochastic") {
Debug.Log("default mode must use either physics or stochastic controller. Defaulting to physics");
action.agentControllerType = "";
SetUpPhysicsController();
}

// if not stochastic, default to physics controller
if (action.agentControllerType.ToLower() == "physics") {
// set up physics controller
SetUpPhysicsController();
}

// if stochastic, set up stochastic controller
else if (action.agentControllerType.ToLower() == "stochastic") {
// set up stochastic controller
primaryAgent.actionFinished(success: false, errorMessage: "Invalid combination of agentControllerType=stochastic and agentMode=default. In order to use agentControllerType=stochastic, agentMode must be set to stochastic");
return;
}
SetUpPhysicsController();
} else if (action.agentMode.ToLower() == "locobot") {
// if not stochastic, default to stochastic
if (action.agentControllerType.ToLower() != "stochastic") {
Debug.Log("'bot' mode only fully supports the 'stochastic' controller type at the moment. Forcing agentControllerType to 'stochastic'");
action.agentControllerType = "stochastic";
}
// LocobotController is a subclass of Stochastic which just the agentMode (VisibilityCapsule) changed
SetUpLocobotController(action);

} else if (action.agentMode.ToLower() == "drone") {
if (action.agentControllerType.ToLower() != "drone") {
Debug.Log("'drone' agentMode is only compatible with 'drone' agentControllerType, forcing agentControllerType to 'drone'");
action.agentControllerType = "drone";
}
SetUpDroneController(action);
} else if (action.agentMode.ToLower() == "stretch") {
} else if (agentMode == "stretch" || agentMode == "arm") {
if (agentMode == "stretch") {
SetUpStretchController(action);

action.autoSimulation = false;
physicsSceneManager.MakeAllObjectsMoveable();

if (action.massThreshold.HasValue) {
if (action.massThreshold.Value > 0.0) {
SetUpMassThreshold(action.massThreshold.Value);
} else {
var error = "massThreshold must have nonzero value - invalid value: " + action.massThreshold.Value;
Debug.Log(error);
primaryAgent.actionFinished(false, error);
return;
}
}
} else if (action.agentMode.ToLower() == "arm") {

if (action.agentControllerType == "") {
action.agentControllerType = "mid-level";
Debug.Log("Defaulting to mid-level.");
} else if (agentMode == "arm") {
SetUpArmController(true);
} else {
// Should not be possible but being very defensive.
throw new ArgumentException($"Invalid agentMode {action.agentMode}");
}

if (action.agentControllerType.ToLower() != "low-level" && action.agentControllerType.ToLower() != "mid-level") {
var error = "'arm' mode must use either low-level or mid-level controller.";
Debug.Log(error);
primaryAgent.actionFinished(success: false, errorMessage: error);
return;
} else if (action.agentControllerType.ToLower() == "mid-level") {
// set up physics controller
SetUpArmController(true);
// the arm should currently be used only with autoSimulation off
// as we manually control Physics during its movement
action.autoSimulation = false;
physicsSceneManager.MakeAllObjectsMoveable();

if (action.massThreshold.HasValue) {
if (action.massThreshold.Value > 0.0) {
SetUpMassThreshold(action.massThreshold.Value);
} else {
var error = "massThreshold must have nonzero value - invalid value: " + action.massThreshold.Value;
Debug.Log(error);
primaryAgent.actionFinished(success: false, errorMessage: error);
return;
}
}
action.autoSimulation = false;
physicsSceneManager.MakeAllObjectsMoveable();
} else {
var error = $"Invalid agentMode {action.agentMode}";
Debug.Log(error);
primaryAgent.actionFinished(success: false, errorMessage: error);
return;
}

if (action.massThreshold.HasValue) {
if (action.massThreshold.Value > 0.0) {
SetUpMassThreshold(action.massThreshold.Value);
} else {
var error = "unsupported";
var error = $"massThreshold must have nonzero value - invalid value: {action.massThreshold.Value}";
Debug.Log(error);
primaryAgent.actionFinished(success: false, errorMessage: error);
primaryAgent.actionFinished(false, error);
return;
}
}
Expand Down Expand Up @@ -1908,7 +1856,6 @@ public class ServerAction {
public float maxDistance;// used in target circle spawning function
public float noise;
public ControllerInitialization controllerInitialization = null;
public string agentControllerType = "";
public string agentMode = "default"; // mode of Agent, valid values are "default" "locobot" "drone", note certain modes are only compatible with certain controller types

public float agentRadius = 2.0f;
Expand Down