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.

Sign up for GitHub

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

Jump to bottom

Revert "Deterministic actions python training" #5622

Merged
merged 1 commit into from
Nov 18, 2021
Merged

Revert "Deterministic actions python training" #5622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 0 additions & 6 deletions com.unity.ml-agents/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,6 @@ and this project adheres to
2. env_params.restarts_rate_limit_n (--restarts-rate-limit-n) [default=1]
3. env_params.restarts_rate_limit_period_s (--restarts-rate-limit-period-s) [default=60]


- Deterministic action selection is now supported during training and inference(#5619)
- Added a new `--deterministic` cli flag to deterministically select the most probable actions in policy. The same thing can
be achieved by adding `deterministic: true` under `network_settings` of the run options configuration.(#5597)
- Extra tensors are now serialized to support deterministic action selection in onnx. (#5593)
- Support inference with deterministic action selection in editor (#5599)
### Bug Fixes
- Fixed a bug where the critics were not being normalized during training. (#5595)
- Fixed the bug where curriculum learning would crash because of the incorrect run_options parsing. (#5586)
Expand Down
4 changes: 1 addition & 3 deletions com.unity.ml-agents/Editor/BehaviorParametersEditor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ internal class BehaviorParametersEditor : UnityEditor.Editor
const string k_BrainParametersName = "m_BrainParameters";
const string k_ModelName = "m_Model";
const string k_InferenceDeviceName = "m_InferenceDevice";
const string k_DeterministicInference = "m_DeterministicInference";
const string k_BehaviorTypeName = "m_BehaviorType";
const string k_TeamIdName = "TeamId";
const string k_UseChildSensorsName = "m_UseChildSensors";
Expand Down Expand Up @@ -69,7 +68,6 @@ public override void OnInspectorGUI()
EditorGUILayout.PropertyField(so.FindProperty(k_ModelName), true);
EditorGUI.indentLevel++;
EditorGUILayout.PropertyField(so.FindProperty(k_InferenceDeviceName), true);
EditorGUILayout.PropertyField(so.FindProperty(k_DeterministicInference), true);
EditorGUI.indentLevel--;
}
needPolicyUpdate = needPolicyUpdate || EditorGUI.EndChangeCheck();
Expand Down Expand Up @@ -158,7 +156,7 @@ void DisplayFailedModelChecks()
{
var failedChecks = Inference.BarracudaModelParamLoader.CheckModel(
barracudaModel, brainParameters, sensors, actuatorComponents,
observableAttributeSensorTotalSize, behaviorParameters.BehaviorType, behaviorParameters.DeterministicInference
observableAttributeSensorTotalSize, behaviorParameters.BehaviorType
);
foreach (var check in failedChecks)
{
Expand Down
6 changes: 2 additions & 4 deletions com.unity.ml-agents/Runtime/Academy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,16 +616,14 @@ void EnvironmentReset()
/// <param name="inferenceDevice">
/// The inference device (CPU or GPU) the ModelRunner will use.
/// </param>
/// <param name="deterministicInference"> Inference only: set to true if the action selection from model should be
/// Deterministic. </param>
/// <returns> The ModelRunner compatible with the input settings.</returns>
internal ModelRunner GetOrCreateModelRunner(
NNModel model, ActionSpec actionSpec, InferenceDevice inferenceDevice, bool deterministicInference = false)
NNModel model, ActionSpec actionSpec, InferenceDevice inferenceDevice)
{
var modelRunner = m_ModelRunners.Find(x => x.HasModel(model, inferenceDevice));
if (modelRunner == null)
{
modelRunner = new ModelRunner(model, actionSpec, inferenceDevice, m_InferenceSeed, deterministicInference);
modelRunner = new ModelRunner(model, actionSpec, inferenceDevice, m_InferenceSeed);
m_ModelRunners.Add(modelRunner);
m_InferenceSeed++;
}
Expand Down
107 changes: 27 additions & 80 deletions com.unity.ml-agents/Runtime/Inference/BarracudaModelExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,8 @@ public static int GetNumVisualInputs(this Model model)
/// <param name="model">
/// The Barracuda engine model for loading static parameters.
/// </param>
/// <param name="deterministicInference"> Inference only: set to true if the action selection from model should be
/// deterministic. </param>
/// <returns>Array of the output tensor names of the model</returns>
public static string[] GetOutputNames(this Model model, bool deterministicInference = false)
public static string[] GetOutputNames(this Model model)
{
var names = new List<string>();

Expand All @@ -124,13 +122,13 @@ public static string[] GetOutputNames(this Model model, bool deterministicInfere
return names.ToArray();
}

if (model.HasContinuousOutputs(deterministicInference))
if (model.HasContinuousOutputs())
{
names.Add(model.ContinuousOutputName(deterministicInference));
names.Add(model.ContinuousOutputName());
}
if (model.HasDiscreteOutputs(deterministicInference))
if (model.HasDiscreteOutputs())
{
names.Add(model.DiscreteOutputName(deterministicInference));
names.Add(model.DiscreteOutputName());
}

var modelVersion = model.GetVersion();
Expand All @@ -151,10 +149,8 @@ public static string[] GetOutputNames(this Model model, bool deterministicInfere
/// <param name="model">
/// The Barracuda engine model for loading static parameters.
/// </param>
/// <param name="deterministicInference"> Inference only: set to true if the action selection from model should be
/// deterministic. </param>
/// <returns>True if the model has continuous action outputs.</returns>
public static bool HasContinuousOutputs(this Model model, bool deterministicInference = false)
public static bool HasContinuousOutputs(this Model model)
{
if (model == null)
return false;
Expand All @@ -164,13 +160,8 @@ public static bool HasContinuousOutputs(this Model model, bool deterministicInfe
}
else
{
bool hasStochasticOutput = !deterministicInference &&
model.outputs.Contains(TensorNames.ContinuousActionOutput);
bool hasDeterministicOutput = deterministicInference &&
model.outputs.Contains(TensorNames.DeterministicContinuousActionOutput);

return (hasStochasticOutput || hasDeterministicOutput) &&
(int)model.GetTensorByName(TensorNames.ContinuousActionOutputShape)[0] > 0;
return model.outputs.Contains(TensorNames.ContinuousActionOutput) &&
(int)model.GetTensorByName(TensorNames.ContinuousActionOutputShape)[0] > 0;
}
}

Expand Down Expand Up @@ -203,10 +194,8 @@ public static int ContinuousOutputSize(this Model model)
/// <param name="model">
/// The Barracuda engine model for loading static parameters.
/// </param>
/// <param name="deterministicInference"> Inference only: set to true if the action selection from model should be
/// deterministic. </param>
/// <returns>Tensor name of continuous action output.</returns>
public static string ContinuousOutputName(this Model model, bool deterministicInference = false)
public static string ContinuousOutputName(this Model model)
{
if (model == null)
return null;
Expand All @@ -216,7 +205,7 @@ public static string ContinuousOutputName(this Model model, bool deterministicIn
}
else
{
return deterministicInference ? TensorNames.DeterministicContinuousActionOutput : TensorNames.ContinuousActionOutput;
return TensorNames.ContinuousActionOutput;
}
}

Expand All @@ -226,10 +215,8 @@ public static string ContinuousOutputName(this Model model, bool deterministicIn
/// <param name="model">
/// The Barracuda engine model for loading static parameters.
/// </param>
/// <param name="deterministicInference"> Inference only: set to true if the action selection from model should be
/// deterministic. </param>
/// <returns>True if the model has discrete action outputs.</returns>
public static bool HasDiscreteOutputs(this Model model, bool deterministicInference = false)
public static bool HasDiscreteOutputs(this Model model)
{
if (model == null)
return false;
Expand All @@ -239,12 +226,7 @@ public static bool HasDiscreteOutputs(this Model model, bool deterministicInfere
}
else
{
bool hasStochasticOutput = !deterministicInference &&
model.outputs.Contains(TensorNames.DiscreteActionOutput);
bool hasDeterministicOutput = deterministicInference &&
model.outputs.Contains(TensorNames.DeterministicDiscreteActionOutput);
return (hasStochasticOutput || hasDeterministicOutput) &&
model.DiscreteOutputSize() > 0;
return model.outputs.Contains(TensorNames.DiscreteActionOutput) && model.DiscreteOutputSize() > 0;
}
}

Expand Down Expand Up @@ -297,10 +279,8 @@ public static int DiscreteOutputSize(this Model model)
/// <param name="model">
/// The Barracuda engine model for loading static parameters.
/// </param>
/// <param name="deterministicInference"> Inference only: set to true if the action selection from model should be
/// deterministic. </param>
/// <returns>Tensor name of discrete action output.</returns>
public static string DiscreteOutputName(this Model model, bool deterministicInference = false)
public static string DiscreteOutputName(this Model model)
{
if (model == null)
return null;
Expand All @@ -310,7 +290,7 @@ public static string DiscreteOutputName(this Model model, bool deterministicInfe
}
else
{
return deterministicInference ? TensorNames.DeterministicDiscreteActionOutput : TensorNames.DiscreteActionOutput;
return TensorNames.DiscreteActionOutput;
}
}

Expand All @@ -336,11 +316,9 @@ public static bool SupportsContinuousAndDiscrete(this Model model)
/// The Barracuda engine model for loading static parameters.
/// </param>
/// <param name="failedModelChecks">Output list of failure messages</param>
///<param name="deterministicInference"> Inference only: set to true if the action selection from model should be
/// deterministic. </param>
///
/// <returns>True if the model contains all the expected tensors.</returns>
/// TODO: add checks for deterministic actions
public static bool CheckExpectedTensors(this Model model, List<FailedCheck> failedModelChecks, bool deterministicInference = false)
public static bool CheckExpectedTensors(this Model model, List<FailedCheck> failedModelChecks)
{
// Check the presence of model version
var modelApiVersionTensor = model.GetTensorByName(TensorNames.VersionNumber);
Expand All @@ -365,9 +343,7 @@ public static bool CheckExpectedTensors(this Model model, List<FailedCheck> fail
// Check the presence of action output tensor
if (!model.outputs.Contains(TensorNames.ActionOutputDeprecated) &&
!model.outputs.Contains(TensorNames.ContinuousActionOutput) &&
!model.outputs.Contains(TensorNames.DiscreteActionOutput) &&
!model.outputs.Contains(TensorNames.DeterministicContinuousActionOutput) &&
!model.outputs.Contains(TensorNames.DeterministicDiscreteActionOutput))
!model.outputs.Contains(TensorNames.DiscreteActionOutput))
{
failedModelChecks.Add(
FailedCheck.Warning("The model does not contain any Action Output Node.")
Expand Down Expand Up @@ -397,51 +373,22 @@ public static bool CheckExpectedTensors(this Model model, List<FailedCheck> fail
}
else
{
if (model.outputs.Contains(TensorNames.ContinuousActionOutput))
if (model.outputs.Contains(TensorNames.ContinuousActionOutput) &&
model.GetTensorByName(TensorNames.ContinuousActionOutputShape) == null)
{
if (model.GetTensorByName(TensorNames.ContinuousActionOutputShape) == null)
{
failedModelChecks.Add(
FailedCheck.Warning("The model uses continuous action but does not contain Continuous Action Output Shape Node.")
);
return false;
}

else if (!model.HasContinuousOutputs(deterministicInference))
{
var actionType = deterministicInference ? "deterministic" : "stochastic";
var actionName = deterministicInference ? "Deterministic" : "";
failedModelChecks.Add(
FailedCheck.Warning($"The model uses {actionType} inference but does not contain {actionName} Continuous Action Output Tensor. Uncheck `Deterministic inference` flag..")
failedModelChecks.Add(
FailedCheck.Warning("The model uses continuous action but does not contain Continuous Action Output Shape Node.")
);
return false;
}
return false;
}

if (model.outputs.Contains(TensorNames.DiscreteActionOutput))
if (model.outputs.Contains(TensorNames.DiscreteActionOutput) &&
model.GetTensorByName(TensorNames.DiscreteActionOutputShape) == null)
{
if (model.GetTensorByName(TensorNames.DiscreteActionOutputShape) == null)
{
failedModelChecks.Add(
FailedCheck.Warning("The model uses discrete action but does not contain Discrete Action Output Shape Node.")
);
return false;
}
else if (!model.HasDiscreteOutputs(deterministicInference))
{
var actionType = deterministicInference ? "deterministic" : "stochastic";
var actionName = deterministicInference ? "Deterministic" : "";
failedModelChecks.Add(
FailedCheck.Warning($"The model uses {actionType} inference but does not contain {actionName} Discrete Action Output Tensor. Uncheck `Deterministic inference` flag.")
failedModelChecks.Add(
FailedCheck.Warning("The model uses discrete action but does not contain Discrete Action Output Shape Node.")
);
return false;
}

return false;
}




}
return true;
}
Expand Down
24 changes: 8 additions & 16 deletions com.unity.ml-agents/Runtime/Inference/BarracudaModelParamLoader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,17 +122,14 @@ public static FailedCheck CheckModelVersion(Model model)
/// <param name="actuatorComponents">Attached actuator components</param>
/// <param name="observableAttributeTotalSize">Sum of the sizes of all ObservableAttributes.</param>
/// <param name="behaviorType">BehaviorType or the Agent to check.</param>
/// <param name="deterministicInference"> Inference only: set to true if the action selection from model should be
/// deterministic. </param>
/// <returns>A IEnumerable of the checks that failed</returns>
public static IEnumerable<FailedCheck> CheckModel(
Model model,
BrainParameters brainParameters,
ISensor[] sensors,
ActuatorComponent[] actuatorComponents,
int observableAttributeTotalSize = 0,
BehaviorType behaviorType = BehaviorType.Default,
bool deterministicInference = false
BehaviorType behaviorType = BehaviorType.Default
)
{
List<FailedCheck> failedModelChecks = new List<FailedCheck>();
Expand All @@ -151,7 +148,7 @@ public static IEnumerable<FailedCheck> CheckModel(
return failedModelChecks;
}

var hasExpectedTensors = model.CheckExpectedTensors(failedModelChecks, deterministicInference);
var hasExpectedTensors = model.CheckExpectedTensors(failedModelChecks);
if (!hasExpectedTensors)
{
return failedModelChecks;
Expand Down Expand Up @@ -184,7 +181,7 @@ public static IEnumerable<FailedCheck> CheckModel(
else if (modelApiVersion == (int)ModelApiVersion.MLAgents2_0)
{
failedModelChecks.AddRange(
CheckInputTensorPresence(model, brainParameters, memorySize, sensors, deterministicInference)
CheckInputTensorPresence(model, brainParameters, memorySize, sensors)
);
failedModelChecks.AddRange(
CheckInputTensorShape(model, brainParameters, sensors, observableAttributeTotalSize)
Expand All @@ -198,7 +195,7 @@ public static IEnumerable<FailedCheck> CheckModel(
);

failedModelChecks.AddRange(
CheckOutputTensorPresence(model, memorySize, deterministicInference)
CheckOutputTensorPresence(model, memorySize)
);
return failedModelChecks;
}
Expand Down Expand Up @@ -321,17 +318,14 @@ ISensor[] sensors
/// The memory size that the model is expecting.
/// </param>
/// <param name="sensors">Array of attached sensor components</param>
/// <param name="deterministicInference"> Inference only: set to true if the action selection from model should be
/// Deterministic. </param>
/// <returns>
/// A IEnumerable of the checks that failed
/// </returns>
static IEnumerable<FailedCheck> CheckInputTensorPresence(
Model model,
BrainParameters brainParameters,
int memory,
ISensor[] sensors,
bool deterministicInference = false
ISensor[] sensors
)
{
var failedModelChecks = new List<FailedCheck>();
Expand Down Expand Up @@ -362,7 +356,7 @@ static IEnumerable<FailedCheck> CheckInputTensorPresence(
}

// If the model uses discrete control but does not have an input for action masks
if (model.HasDiscreteOutputs(deterministicInference))
if (model.HasDiscreteOutputs())
{
if (!tensorsNames.Contains(TensorNames.ActionMaskPlaceholder))
{
Expand All @@ -382,19 +376,17 @@ static IEnumerable<FailedCheck> CheckInputTensorPresence(
/// The Barracuda engine model for loading static parameters
/// </param>
/// <param name="memory">The memory size that the model is expecting/</param>
/// <param name="deterministicInference"> Inference only: set to true if the action selection from model should be
/// deterministic. </param>
/// <returns>
/// A IEnumerable of the checks that failed
/// </returns>
static IEnumerable<FailedCheck> CheckOutputTensorPresence(Model model, int memory, bool deterministicInference = false)
static IEnumerable<FailedCheck> CheckOutputTensorPresence(Model model, int memory)
{
var failedModelChecks = new List<FailedCheck>();

// If there is no Recurrent Output but the model is Recurrent.
if (memory > 0)
{
var allOutputs = model.GetOutputNames(deterministicInference).ToList();
var allOutputs = model.GetOutputNames().ToList();
if (!allOutputs.Any(x => x == TensorNames.RecurrentOutput))
{
failedModelChecks.Add(
Expand Down
Loading