-
Notifications
You must be signed in to change notification settings - Fork 4.3k
[MLA-1981] Don't allow connections to newer python trainers #5370
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
m_EditorVersion: 2018.4.17f1 | ||
m_EditorVersion: 2018.4.32f1 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Text.RegularExpressions; | ||
using UnityEngine; | ||
using Unity.MLAgents.Analytics; | ||
using Unity.MLAgents.CommunicatorObjects; | ||
|
@@ -99,6 +100,41 @@ internal static bool CheckCommunicationVersionsAreCompatible( | |
} | ||
|
||
internal static bool CheckPythonPackageVersionIsCompatible(string pythonLibraryVersion) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The diff is messy; the old code was renamed to CheckPythonPackageVersionIsSupported. |
||
{ | ||
// Try to extract the numerical values from the pythonLibraryVersion, e.g. remove the ".dev0" suffix | ||
var versionMatch = Regex.Match(pythonLibraryVersion, @"[0-9]+\.[0-9]+\.[0-9]"); | ||
if (versionMatch.Success) | ||
{ | ||
pythonLibraryVersion = versionMatch.Value; | ||
} | ||
|
||
Version pythonVersion; | ||
try | ||
{ | ||
pythonVersion = new Version(pythonLibraryVersion); | ||
} | ||
catch | ||
{ | ||
// Unparseable version. | ||
// Anything like 0.42.0.dev0 should have been caught with the regex above, so anything here | ||
// is totally bogus. For now, ignore these and let CheckPythonPackageVersionIsSupported handle it. | ||
return true; | ||
} | ||
|
||
if (pythonVersion > PythonTrainerVersions.s_MaxCompatibleVersion) | ||
{ | ||
return false; | ||
} | ||
return true; | ||
} | ||
|
||
/// <summary> | ||
/// Check if the package is in the supported range. Note that some versions might be unsupported but | ||
/// still compatible. | ||
/// </summary> | ||
/// <param name="pythonLibraryVersion"></param> | ||
/// <returns></returns> | ||
internal static bool CheckPythonPackageVersionIsSupported(string pythonLibraryVersion) | ||
{ | ||
Version pythonVersion; | ||
try | ||
|
@@ -107,7 +143,7 @@ internal static bool CheckPythonPackageVersionIsCompatible(string pythonLibraryV | |
} | ||
catch | ||
{ | ||
// Unparseable - this also catches things like "0.20.0-dev0" which we don't want to support | ||
// Unparseable - this also catches things like "0.20.0.dev0" which we don't want to support | ||
return false; | ||
} | ||
|
||
|
@@ -182,7 +218,21 @@ public UnityRLInitParameters Initialize(CommunicatorInitParameters initParameter | |
throw new UnityAgentsException("ICommunicator.Initialize() failed."); | ||
} | ||
|
||
var packageVersionSupported = CheckPythonPackageVersionIsCompatible(pythonPackageVersion); | ||
var packageVersionCompatible = CheckPythonPackageVersionIsCompatible(pythonPackageVersion); | ||
if (!packageVersionCompatible) | ||
{ | ||
Debug.LogErrorFormat( | ||
"Python package version ({0}) will produce model files that are incompatible with this " + | ||
"version of the com.unity.ml-agents Unity package. Please downgrade to a Python package " + | ||
"between {1} and {2}, or update to a new version of com.unity.ml-agents.", | ||
pythonPackageVersion, | ||
PythonTrainerVersions.s_MinSupportedVersion, | ||
PythonTrainerVersions.s_MaxSupportedVersion | ||
); | ||
throw new UnityAgentsException("Incompatible trainer version."); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This gets caught and a different exception is thrown below. |
||
} | ||
|
||
var packageVersionSupported = CheckPythonPackageVersionIsSupported(pythonPackageVersion); | ||
if (!packageVersionSupported) | ||
{ | ||
Debug.LogWarningFormat( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,10 +13,14 @@ internal enum MenuGroup | |
|
||
internal static class PythonTrainerVersions | ||
{ | ||
// The python package version must be >= s_MinSupportedVersion | ||
// The python package version should be >= s_MinSupportedVersion | ||
// and <= s_MaxSupportedVersion. | ||
internal static Version s_MinSupportedVersion = new Version("0.16.1"); | ||
internal static Version s_MaxSupportedVersion = new Version("0.20.0"); | ||
|
||
// Any version > to this is known to be incompatible and we will block training. | ||
// Covers any patch to the release before the 2.0.0 package release. | ||
internal static Version s_MaxCompatibleVersion = new Version("0.25.999"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just in case we do a 0.25.2 patch release. |
||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This (and the manifest) were pretty old. Just affects Project, but it's annoying to keep excluding them.