-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Always reset when agent is done #3222
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
Conversation
m_VectorSensorBuffer = new float[sensors.GetSensorFloatObservationSize()]; | ||
} | ||
|
||
// This is a bit of a hack - if we're in inference mode, observations won't be generated |
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.
Copied comment doesn't make sense here.
…logies/ml-agents into develop-agentondone
// This is a bit of a hack - if we're in inference mode, observations won't be generated | ||
// But we need these to be generated for the recorder. So generate them here. | ||
var observations = new List<Observation>(); | ||
GenerateSensorData(sensors, m_VectorSensorBuffer, m_WriteAdapter, observations); |
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.
I'm confused - you said before that the problem was some of the agent sensors might not be availble for generation (e.g. camera already cleaned up). If that's the case, won't that still be a problem here? How's this different from passing the actual sensors to RequestDecision?
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.
We generate the data earlier. OnDisable is called first on all destroyed gameObjects and then On Destroy is called on all game objects.
We call GenerateSensorData in OnDisable rather than in the next fixed update.
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.
I'm generally confused about what this function is attempting to achieve.
Migration guide? Sounds OK otherwise. |
* fill 0s for done agents * docstrings
Re-requesting review since a lot changed |
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.
Looks good. I think we can remove Agent.GenerateSensorData() now, but that can happen in another PR. Never mind, you did.
Maybe "Always reset Agent on Done" would be a user-friendlier PR title?
Co-Authored-By: Chris Elion <chris.elion@unity3d.com>
In this PR:
AgentOnDone
virtual method on theAgent
classReset On Done
checkbox from theAgent
inspectorFixedSensor
that takes as input anObservation
and will always return that observation when calledIPolicy
with a true done flag, a list ofFixedSensor
and an empty callback.AgentOnDone