Goal conditioning grid world : Example of goal conditioning

[vincentpierre]

Proposed change(s)

Making GridWorld use the new goal conditioning.

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

[chriselion]

Can this happen somewhere else? It feels like abuse of CollectObservations(), since it's not touching the input VectorSensor.

[vincentpierre]

VectorSensor is null here, I do not see an issue with this. Goal Signal is an observation, so it makes sense to me that it is called in CollectObservation.
Would it be better if I put this logic into a CollectGoal method with no arguments that I call in CollectObservations ?

[chriselion]

CollectGoal is maybe for the example (but let's not add it Agent). Let me think about a better way.

One problem (which I didn't realize until now) is that we don't check for null CollectObservationsSensor during the normal update step:

ml-agents/com.unity.ml-agents/Runtime/Agent.cs

Line 1062 in e4e9c51

CollectObservations(collectObservationsSensor);

but we do check for null when the agent is done:

ml-agents/com.unity.ml-agents/Runtime/Agent.cs

Lines 563 to 571 in e4e9c51

	if (collectObservationsSensor != null)
	{
	// Make sure the latest observations are being passed to training.
	collectObservationsSensor.Reset();
	using (m_CollectObservationsChecker.Start())
	{
	CollectObservations(collectObservationsSensor);
	}
	}

[chriselion]

This is pretty confusing since the "goal" tag doesn't really mean that it's the goal anymore. Can you change them to e.g. "plus" and "ex"?

Or maybe this would be a good opportunity to stop using physics collision checks, and change the example to use a 2D array of enums? That would probably speed up training too.

[vincentpierre]

I changed the tags to plus and ex. I think making the grid a 2D array of enums is a good idea, but out of scope for this.

[chriselion]

(sorry, can't comment inline for the file removal). gridworld.png is still being referenced:

$ git grep gridworld.png 
docs/Learning-Environment-Design-Agents.md:![Agent RenderTexture Debug](images/gridworld.png)
docs/Learning-Environment-Examples.md:![GridWorld](images/gridworld.png)

(and that should have failed the link checker)

[vincentpierre]

gridworld.png is still being referenced

gridworld.png is still there (It is only smaller)

[ervteng]

Might not be related to this PR, but should we add a warning in the docs about using hypernetworks for larger hidden_units values? We might even be able to auto-detect it in settings.py, e.g. if the resulting model will be bigger than 50mb print a warning

[vincentpierre]

Might not be related to this PR, but should we add a warning in the docs about using hypernetworks for larger hidden_units values? We might even be able to auto-detect it in settings.py, e.g. if the resulting model will be bigger than 50mb print a warning

There is this line in the documentation:

If set to `hyper` (default) a [HyperNetwork](https://arxiv.org/pdf/1609.09106.pdf)
will be used to generate some of the
weights of the policy using the goal observations as input. Note that using a
HyperNetwork requires a lot of computations, it is recommended to use a smaller
number of hidden units in the policy to alleviate this.

I am hesitant to throw a warning if the model is going to be large because we never know what the user has in mind...

[ervteng]

Possible to link to this environment in the goal signal docs and the Changelog? Just in case a user wants an example of how to use these features