Hybrid FoodCollector #4746
Hybrid FoodCollector #4746
Conversation
| @@ -139,16 +129,7 @@ public void MoveAgent(ActionSegment<int> act) | |||
| if (shootCommand) | |||
| { | |||
| m_Shoot = true; | |||
| dirToGo *= 0.5f; | |||
There was a problem hiding this comment.
This got lost in the conversion, right?
There was a problem hiding this comment.
Maybe easier to keep everything in one function and just pass the ActionBuffers, instead of doing it individually.
| if (Input.GetKey(KeyCode.D)) | ||
| { | ||
| discreteActionsOut[2] = 2; | ||
| continuousActionsOut[2] = 2; |
There was a problem hiding this comment.
These should be 1 and -1 (or the other way around), not 1 and 2
| - `FoodCollector` scene: 3 Continuous Actions + 1 Discrete Action with 2 Branches | ||
| - 3 continuous actions correspond to Forward Motion, Side Motion and Rotation | ||
| - Laser (2 possible actions: Laser, No Action) | ||
| - `GridFoodCollector` and `VisualFoodCollector` scenes: (Discrete) 4 Branches: |
There was a problem hiding this comment.
I think we either need to change the ActionSpecs for the other scenes (and retrain), or have two versions of the code to handle discrete or continuous movement.
There was a problem hiding this comment.
I'd prefer the former (less code to maintain) but it's your call.
| - `FoodCollector` scene: 3 Continuous Actions + 1 Discrete Action with 2 Branches | ||
| - 3 continuous actions correspond to Forward Motion, Side Motion and Rotation | ||
| - Laser (2 possible actions: Laser, No Action) | ||
| - `GridFoodCollector` and `VisualFoodCollector` scenes: (Discrete) 4 Branches: |
There was a problem hiding this comment.
Should these be the same as above since they share the FoodCollectorAgent script?
| ) | ||
| if ( | ||
| current_pair_info.action_info.continuous_actions is None | ||
| and current_pair_info.action_info.continuous_actions is None |
There was a problem hiding this comment.
| and current_pair_info.action_info.continuous_actions is None | |
| and current_pair_info.action_info.discrete_actions is None |
(I think, not totally sure)
| @@ -23,9 +24,9 @@ internal class DemonstrationEditor : UnityEditor.Editor | |||
| const string k_NumberStepsName = "numberSteps"; | |||
| const string k_NumberEpisodesName = "numberEpisodes"; | |||
| const string k_MeanRewardName = "meanReward"; | |||
| const string k_ActionSpecName = "ActionSpec"; | |||
| const string k_ActionSpecName = "m_ActionSpec"; | |||
There was a problem hiding this comment.
What was the problem here? Should we have a unit test that would have caught it?
(OK to do outside of this PR)
There was a problem hiding this comment.
I wrote this when we were adding an ActionSpec field, and later on we changed it to private field m_ActionSpec and public property ActionSpec.
Since properties won't be serialized, property.FindPropertyRelative("ActionSpec") will get nothing and the demo import will fail. The exception is ignored though, resulted in demo files not imported properly and no information displayed in inspector, but no direct error message.
There was a problem hiding this comment.
Ah sorry, missed the fact that it was in the drawer.
* use continuous action for moving and discrete for shooting * update models
Proposed change(s)
Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)
Types of change(s)
Checklist
Other comments