Skip to content

Add frozen flag to VisualFoodCollector #4511

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

Merged
merged 11 commits into from
Oct 6, 2020
Merged

Add frozen flag to VisualFoodCollector #4511

merged 11 commits into from
Oct 6, 2020

Conversation

dongruoping
Copy link
Contributor

@dongruoping dongruoping commented Sep 25, 2020

Proposed change(s)

The FoodCollector environment does not train with pure visual observation due to insufficient information. The agent lacks of information about the frozen state of itself, and adding a boolean flag indicating the frozen state (which is also used in the vector observation version of FoodCollector) makes the training work.

This will add an example of using both vector and visual observation to our example environments.

Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)

Types of change(s)

  • Bug fix
  • New feature
  • Code refactor
  • Breaking change
  • Documentation update
  • Other (please describe)

Checklist

  • Added tests that prove my fix is effective or that my feature works
  • Updated the changelog (if applicable)
  • Updated the documentation (if applicable)
  • Updated the migration guide (if applicable)

Other comments

@dongruoping
Copy link
Contributor Author

Docs and changelogs WIP

Copy link
Contributor

@vincentpierre vincentpierre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you verified that it trains, this is great news!
I left a comment, please address before merging.
do you plan on adding this scene to the daily CI?

@@ -30,6 +30,7 @@ public class FoodCollectorAgent : Agent
public GameObject myLaser;
public bool contribute;
public bool useVectorObs;
public bool useFrozenFlag;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should always be turned on. The user might have issues with the number of observations when unchecking this box. Besides, you mentioned that the Agent will not train without it, so I think it should be always on and un-uncheckable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern was that it would be a bit confusing to use one vector flag by default in a "visual" scene.
Or is it actually not too big a problem? if so I can make it un-uncheckable.

@dongruoping
Copy link
Contributor Author

Yes I think it would be good to be include in CI, so that we can capture it if we change the way combining vector and visual observations. It could take quite some time though.
FYI: Using the current config, the reward bounce between -1~1 in the first 1.5M steps, get to ~20 at around 2M steps and ~50 around 3M steps.

@dongruoping
Copy link
Contributor Author

Added the trained model and updated the docs, so I think it'd be better to do re-review

Copy link
Contributor

@vincentpierre vincentpierre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks, good to me. I still think the useFrozenFlag should not be an option and should always be on. I tried the model and it worked. I did not try to train it.

Ruo-Ping (Rachel) Dong and others added 4 commits October 5, 2020 13:38
@dongruoping
Copy link
Contributor Author

Looks, good to me. I still think the useFrozenFlag should not be an option and should always be on. I tried the model and it worked. I did not try to train it.

Removed useFrozenFlag as suggested and verified the change does not affect the model.

@dongruoping dongruoping merged commit a261b40 into master Oct 6, 2020
@delete-merged-branch delete-merged-branch bot deleted the develop-vis-food branch October 6, 2020 21:02
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants