Skip to content
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

[MLA-488] Fix observations in demonstration drawer #3771

Merged
merged 6 commits into from
Apr 14, 2020

Conversation

chriselion
Copy link
Contributor

@chriselion chriselion commented Apr 13, 2020

Proposed change(s)

Fix a long-standing bug in the DemonstrationDrawer. It was never updated to handle the changes to Observations.

PushBlock demo, which has multiple 1-D sensors:
image

GridWorld demo, which has 1 multi-D sensor:
image

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

https://jira.unity3d.com/browse/MLA-488

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

@chriselion chriselion changed the title Mla 488 demonstration drawer [MLA-488] Fix observations in demonstration drawer Apr 13, 2020
@chriselion chriselion requested review from surfnerd and vincentpierre and removed request for surfnerd April 13, 2020 23:18
/// </summary>
/// <param name="bpp">An instance of a brain parameters protobuf object.</param>
/// <returns>A BrainParameters struct.</returns>
public static BrainParameters ToBrainParameters(this BrainParametersProto bpp)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just moved this closer to the BrainParameters -> BrainParametersProto conversion

/// Used for imitation learning, or other forms of learning from data.
/// </summary>
[Serializable]
internal class Demonstration : ScriptableObject
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Split this file into DemonstrationSummary.cs and DemonstrationMetaData.cs

@chriselion
Copy link
Contributor Author

Old inspector for comparison:
image

@chriselion chriselion marked this pull request as ready for review April 13, 2020 23:38
[Serializable]
internal class DemonstrationSummary : ScriptableObject
{
public DemonstrationMetaData metaData;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the DemonstrationSummary need DemonstrationMetaData ? I thought the two were side by side anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The fields from the DemonstrationMetaData are displayed in the inspector. DemonstrationSummary is only used by the DemonstrationDrawer, it's not a full Demonstration (that's why I renamed it).

@@ -4,7 +4,7 @@ ScriptedImporter:
fileIDToRecycleName:
11400000: Assets/ML-Agents/Examples/Bouncer/Demos/ExpertBouncer.demo
externalObjects: {}
userData: ' (MLAgents.Demonstrations.Demonstration)'
userData: ' (MLAgents.Demonstrations.DemonstrationSummary)'
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that old demo files will be incompatible (nothing wrong with that, just making sure)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, shouldn't have any effect.

userData = demonstration.ToString();
var demonstrationSummary = ScriptableObject.CreateInstance<DemonstrationSummary>();
demonstrationSummary.Initialize(brainParameters, metaData, observationSummaries);
userData = demonstrationSummary.ToString();
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this variable used ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not really sure about the purpose of it. It sets this field https://docs.unity3d.com/ScriptReference/AssetImporter-userData.html which explains the .meta file changes.

@chriselion chriselion merged commit 1e164cc into master Apr 14, 2020
@delete-merged-branch delete-merged-branch bot deleted the MLA-488-DemonstrationDrawer branch April 14, 2020 20:08
mmattar pushed a commit that referenced this pull request Apr 15, 2020
commit 9600d0f
Author: Marwan Mattar <marwan@unity3d.com>
Date:   Tue Apr 14 17:45:33 2020 -0700

    Various doc improvements (#3775)

    * Various doc improvements

    For Using-Virtual-Environment.md:
    - Made a note regarding updating setuptools and pip.
    - Changed lists from "-" to "*"

    For Using-Tensorboard.md:
    - Changed the ordered list to use "1."

    For Training-on-Microsoft-Azure-Custom-Instance.md:
    - Deleted as it was not linked anywhere

    For FAQ.md
    - Removed stale issues given upgrade to 2018.3

    For Readme.md
    - Added links for Reward Signals, Self-Play and Profiling Trainers

    For Learning-Environment-Executable.md
    - Changed the ordered list to use "1."

    For Learning-Environment-Examples.md
    - Minor rewording of intro paragraphs

    * consolidating custom instances page in main page

    So we have a single page for Azure.

    Adding warning note for deprecated docs

    * Fixing doc links that are failing CI

commit 34db9c2
Author: Vincent-Pierre BERGES <vincentpierre@unity3d.com>
Date:   Tue Apr 14 14:06:16 2020 -0700

    Removing dark mode from screenshots (#3772)

commit 1e164cc
Author: Chris Elion <chris.elion@unity3d.com>
Date:   Tue Apr 14 13:08:18 2020 -0700

    [MLA-488] Fix observations in demonstration drawer (#3771)

    * WIP observation shapes

    * WIP observation shapes

    * fix Observation shape serialization

    * rename field, redo screenshot

    * docstring

    * changelog
mmattar pushed a commit that referenced this pull request Apr 15, 2020
* Improvements to Training-ML-Agents

- Removed duplicate documentation
- Moved CLI descriptions to learn.py
- Reorganized "Training with mlagents-learn" into 5 sub-sections

* fixed formatting errors and incorporated minor feedback

* minor improvement

* Minor formatting.

* fixed run-id references

* Keeping link to use Inference consistent with master

Will update the UIE page in a separate PR.

* Squashed commit of the following:

commit 9600d0f
Author: Marwan Mattar <marwan@unity3d.com>
Date:   Tue Apr 14 17:45:33 2020 -0700

    Various doc improvements (#3775)

    * Various doc improvements

    For Using-Virtual-Environment.md:
    - Made a note regarding updating setuptools and pip.
    - Changed lists from "-" to "*"

    For Using-Tensorboard.md:
    - Changed the ordered list to use "1."

    For Training-on-Microsoft-Azure-Custom-Instance.md:
    - Deleted as it was not linked anywhere

    For FAQ.md
    - Removed stale issues given upgrade to 2018.3

    For Readme.md
    - Added links for Reward Signals, Self-Play and Profiling Trainers

    For Learning-Environment-Executable.md
    - Changed the ordered list to use "1."

    For Learning-Environment-Examples.md
    - Minor rewording of intro paragraphs

    * consolidating custom instances page in main page

    So we have a single page for Azure.

    Adding warning note for deprecated docs

    * Fixing doc links that are failing CI

commit 34db9c2
Author: Vincent-Pierre BERGES <vincentpierre@unity3d.com>
Date:   Tue Apr 14 14:06:16 2020 -0700

    Removing dark mode from screenshots (#3772)

commit 1e164cc
Author: Chris Elion <chris.elion@unity3d.com>
Date:   Tue Apr 14 13:08:18 2020 -0700

    [MLA-488] Fix observations in demonstration drawer (#3771)

    * WIP observation shapes

    * WIP observation shapes

    * fix Observation shape serialization

    * rename field, redo screenshot

    * docstring

    * changelog

* Revert "Squashed commit of the following:"

This reverts commit 819e8d5.

* Removing debugging section.

* Prettier formatting

* Fixing Prettier oddity
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 15, 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