-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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.
| // 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 <[email protected]>
In this PR:
AgentOnDonevirtual method on theAgentclassReset On Donecheckbox from theAgentinspectorFixedSensorthat takes as input anObservationand will always return that observation when calledIPolicywith a true done flag, a list ofFixedSensorand an empty callback.AgentOnDone