Make BehaviorParameters public again, update editor #3595
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chriselion
commented
Mar 9, 2020
chriselion
commented
Mar 9, 2020
chriselion
commented
Mar 9, 2020
com.unity.ml-agents/Runtime/Agent.cs
Outdated
| @@ -362,36 +364,49 @@ void NotifyAgentDone(DoneReason doneReason) | |||
| /// Updates the Model for the agent. Any model currently assigned to the | |||
| /// agent will be replaced with the provided one. If the arguments are | |||
| /// identical to the current parameters of the agent, the model will | |||
| /// remain unchanged. | |||
| /// remain unchanged, unless the force parameter is true. | |||
There was a problem hiding this comment.
The "If the arguments are identical..." comment was wrong before :)
chriselion
commented
Mar 9, 2020
chriselion
commented
Mar 9, 2020
com.unity.ml-agents/Runtime/Agent.cs
Outdated
| { | ||
| if (m_PolicyFactory.m_BehaviorType == behaviorType) | ||
| if (m_PolicyFactory.behaviorType == behaviorType && !force) |
There was a problem hiding this comment.
Same idea here behind force.
There was a problem hiding this comment.
Would exposing an internal "Reload Policy" do the trick ?
internal void ReloadPolicy()
{
m_Brain?.Dispose();
m_Brain = m_PolicyFactory.GeneratePolicy(Heuristic);
}There was a problem hiding this comment.
Ah yes, that would be a lot cleaner! Then BehaviorParametersEditor can just call that.
chriselion
commented
Mar 9, 2020
chriselion
commented
Mar 9, 2020
| public bool useChildSensors | ||
| { | ||
| get { return m_UseChildSensors; } | ||
| internal set { m_UseChildSensors = value; } // TODO make public, don't allow changes at runtime |
There was a problem hiding this comment.
Currently internal since it affects the sensor shapes.
chriselion
commented
Mar 9, 2020
| @@ -105,7 +133,7 @@ public string fullyQualifiedBehaviorName | |||
| get { return m_BehaviorName + "?team=" + TeamId; } | |||
| } | |||
|
|
|||
| public IPolicy GeneratePolicy(Func<float[]> heuristic) | |||
| internal IPolicy GeneratePolicy(Func<float[]> heuristic) | |||
There was a problem hiding this comment.
Had to make this internal since IPolicy is internal.
chriselion
commented
Mar 9, 2020
chriselion
commented
Mar 9, 2020
| @@ -38,7 +38,7 @@ internal void SetPolicy(IPolicy policy) | |||
|
|
|||
| internal IPolicy GetPolicy() | |||
| { | |||
| return (IPolicy) typeof(Agent).GetField("m_Brain", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(this); | |||
| return (IPolicy)typeof(Agent).GetField("m_Brain", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(this); | |||
There was a problem hiding this comment.
This got auto-formatted.
surfnerd
reviewed
Mar 9, 2020
surfnerd
reviewed
Mar 9, 2020
surfnerd
approved these changes
Mar 9, 2020
com.unity.ml-agents/Runtime/Agent.cs
Outdated
| { | ||
| if (m_PolicyFactory.m_BehaviorType == behaviorType) | ||
| if (m_PolicyFactory.behaviorType == behaviorType && !force) |
There was a problem hiding this comment.
Would exposing an internal "Reload Policy" do the trick ?
internal void ReloadPolicy()
{
m_Brain?.Dispose();
m_Brain = m_PolicyFactory.GeneratePolicy(Heuristic);
}
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed change(s)
Make BehaviorParameters and BrainParameters public again.
Update the BehaviorParametersEditor to push changes (e.g. model, behavior type) to the Agent at runtime.
Some property setters are still
internal; this currently indicates the property is not safe to change during training/inference. I will do a pass on these in a subsequent PR to make the public and make sure they have effect at runtime.Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)
Will (mostly) resolve #3580
https://jira.unity3d.com/browse/MLA-741
Types of change(s)
Checklist
Other comments