Skip to content

better logging for ports and versions #3048

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

Merged
merged 1 commit into from
Dec 10, 2019
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
42 changes: 27 additions & 15 deletions UnitySDK/Assets/ML-Agents/Scripts/Academy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ namespace MLAgents
public abstract class Academy : MonoBehaviour
{
const string k_ApiVersion = "API-12";
const int k_EditorTrainingPort = 5004;

/// Temporary storage for global gravity value
/// Used to restore oringal value when deriving Academy modifies it
Expand Down Expand Up @@ -149,7 +150,7 @@ public void LazyInitialization()
}

// Used to read Python-provided environment parameters
static int ReadArgs()
static int ReadPortFromArgs()
{
var args = System.Environment.GetCommandLineArgs();
var inputPort = "";
Expand All @@ -161,7 +162,22 @@ static int ReadArgs()
}
}

return int.Parse(inputPort);
try
{
return int.Parse(inputPort);
}
catch
{
// No arg passed, or malformed port number.
#if UNITY_EDITOR
// Try connecting on the default editor port
return k_EditorTrainingPort;
#else
// This is an executable, so we don't try to connect.
return -1;
#endif
}

}

/// <summary>
Expand All @@ -179,23 +195,15 @@ void InitializeEnvironment()


// Try to launch the communicator by using the arguments passed at launch
try
var port = ReadPortFromArgs();
if (port > 0)
{
Communicator = new RpcCommunicator(
new CommunicatorInitParameters
{
port = ReadArgs()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The try/catch here was really for the ReadArgs(); RpcCommunicator never throws.

});
}
catch
{
#if UNITY_EDITOR
Communicator = new RpcCommunicator(
new CommunicatorInitParameters
{
port = 5004
});
#endif
port = port
}
);
}

if (Communicator != null)
Expand All @@ -217,6 +225,10 @@ void InitializeEnvironment()
}
catch
{
Debug.Log($"" +
$"Couldn't connect to trainer on port {port} using API version {k_ApiVersion}. " +
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hope this is OK, I want some information to make sure we're using the same port as python, but also want to make it clear that this is expected sometimes (when doing inference).

"Will perform inference instead."
);
Communicator = null;
}

Expand Down
10 changes: 6 additions & 4 deletions ml-agents-envs/mlagents/envs/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ def __init__(
self.executable_launcher(file_name, docker_training, no_graphics, args)
else:
logger.info(
"Start training by pressing the Play button in the Unity Editor."
f"Listening on port {self.port}. "
f"Start training by pressing the Play button in the Unity Editor."
)
self._loaded = True

Expand All @@ -119,9 +120,10 @@ def __init__(
if self._unity_version != self._version_:
self._close()
raise UnityEnvironmentException(
"The API number is not compatible between Unity and python. Python API : {0}, Unity API : "
"{1}.\nPlease go to https://github.com/Unity-Technologies/ml-agents to download the latest version "
"of ML-Agents.".format(self._version_, self._unity_version)
f"The API number is not compatible between Unity and python. "
f"Python API: {self._version_}, Unity API: {self._unity_version}.\n"
f"Please go to https://github.com/Unity-Technologies/ml-agents/releases/tag/latest_release"
f"to download the latest version of ML-Agents."
)
self._n_agents: Dict[str, int] = {}
self._is_first_message = True
Expand Down
14 changes: 7 additions & 7 deletions ml-agents/mlagents/trainers/learn.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,11 @@ def from_argparse(args: Any) -> "CommandLineOptions":


def get_version_string() -> str:
return f""" Version information:\n
ml-agents: {mlagents.trainers.__version__},
ml-agents-envs: {mlagents.envs.__version__},
Communicator API: {UnityEnvironment.API_VERSION},
TensorFlow: {tf_utils.tf.__version__}
"""
return f""" Version information:
ml-agents: {mlagents.trainers.__version__},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just trimmed up the whitespace here.

ml-agents-envs: {mlagents.envs.__version__},
Communicator API: {UnityEnvironment.API_VERSION},
TensorFlow: {tf_utils.tf.__version__}"""


def parse_command_line(argv: Optional[List[str]] = None) -> CommandLineOptions:
Expand Down Expand Up @@ -170,7 +169,7 @@ def parse_command_line(argv: Optional[List[str]] = None) -> CommandLineOptions:
"--cpu", default=False, action="store_true", help="Run with CPU only"
)

parser.add_argument("--version", action="version", version=get_version_string())
parser.add_argument("--version", action="version", version="")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--version still works, but we always print the version anyway. I think it's worth keeping since that's a standard argument for a CLI.


eng_conf = parser.add_argument_group(title="Engine Configuration")
eng_conf.add_argument(
Expand Down Expand Up @@ -438,6 +437,7 @@ def main():
)
except Exception:
print("\n\n\tUnity Technologies\n")
print(get_version_string())
options = parse_command_line()
trainer_logger = logging.getLogger("mlagents.trainers")
env_logger = logging.getLogger("mlagents.envs")
Expand Down