Skip to content

Develop side channels: migrate reset parameters#2990

Merged
vincentpierre merged 45 commits into
developfrom
develop-side-channel-2
Dec 4, 2019
Merged

Develop side channels: migrate reset parameters#2990
vincentpierre merged 45 commits into
developfrom
develop-side-channel-2

Conversation

@vincentpierre
Copy link
Copy Markdown
Contributor

@vincentpierre vincentpierre commented Nov 27, 2019

TODO before merging

Documentation (Will do in another PR)

Feature changes :

One on the main breaking changes is that curriculum no longer checks if the reset parameters are in the environment (since parameters can now be added during the simulation)

vincentpierre and others added 30 commits November 21, 2019 17:34
* Added the side channel for the Engine Configuration.

Note that this change does not require modifying a lot of files :
 - Adding a sender in Python
 - Adding a receiver in C#
 - subscribe the receiver to the communicator (here is a one liner in the Academy)
 - Add the side channel to the Python UnityEnvironment (not represented here)

Adding the side channel to the environment would look like such :

```python
from mlagents.envs.environment import UnityEnvironment
from mlagents.envs.side_channel.raw_bytes_channel import RawBytesChannel
from mlagents.envs.side_channel.engine_configuration_channel import EngineConfigurationChannel

channel0 = RawBytesChannel()
channel1 = EngineConfigurationChannel()

env = UnityEnvironment(base_port = 5004, side_channels = [channel0, channel1])
```

* renamings

* addressing comments
Co-Authored-By: Chris Elion <chris.elion@unity3d.com>
Co-Authored-By: Chris Elion <chris.elion@unity3d.com>
Comment thread ml-agents-envs/mlagents/envs/subprocess_env_manager.py Outdated
SubprocessEnvManager.create_worker = MagicMock()
env = SubprocessEnvManager(mock_env_factory, 2)
env = SubprocessEnvManager(
mock_env_factory, EngineConfig(80, 80, 1, 20.0, -1), 2
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about adding a .default_config() staticmethod to EngineConfig? (yes, you can do that to NamedTuples)

Copy link
Copy Markdown
Contributor

@chriselion chriselion 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! Can you make sure you update the docs:

@vincentpierre
Copy link
Copy Markdown
Contributor Author

@chriselion

Looks good! Can you make sure you update the docs

There are quite a few changes to the docs. I will make a PR to this branch with the docs edits and merge this afterwards.

* Docs changes for the Side Channels feature

* replace deprecated with removed on the CustomResetPratmeters`

* Update docs/Python-API.md

Co-Authored-By: Chris Elion <chris.elion@unity3d.com>

* Update docs/Training-Generalized-Reinforcement-Learning-Agents.md

Co-Authored-By: Chris Elion <chris.elion@unity3d.com>

* Removing the console outputs in the docs

* Update docs/Training-ML-Agents.md

Co-Authored-By: Chris Elion <chris.elion@unity3d.com>

* replace does not work with ignored

* adding a note on side channels

* adding some steps to migrate

* addressing comments

* adding more docs to the LL-API

* added a blob on how to access the properties in C#

* adding space between ResetParameters

* fix typo
public void SetLaserLengths()
{
m_LaserLength = m_MyAcademy.resetParameters.TryGetValue("laser_length", out m_LaserLength) ? m_LaserLength : 1.0f;
m_LaserLength = m_MyAcademy.FloatProperties.GetPropertyWithDefault("laser_length", 1.0f) * m_LaserLength;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think you want * m_LaserLength here.


if (Communicator != null)
{
Communicator.RegisterSideChannel(new EngineConfigurationChannel());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Doesn't have to be in this PR, but we should think some more about how we expect users to register their own side channels. Might need a virtual call here that they can override.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We could also go the route of having a side channel component (a little bit like what you did with the sensors)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like that idea, though if the academy won't be in the scene in the future, maybe that doesn't make sense. I'm not sure though.

if (m_IsInference)
{
ConfigureEnvironmentHelper(m_InferenceConfiguration);
Monitor.SetActive(true);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've never used the Monitor so I don't know much about it, but these were the only places we're calling SetActive on it. Do these calls need to be moved somewhere?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The monitor is on by default now, regardless of wether we are training or not.

Comment thread UnitySDK/Assets/ML-Agents/Scripts/SideChannel/EngineConfigurationChannel.cs Outdated

parameters = self.data["parameters"]
for key in parameters:
if key not in default_reset_parameters:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we have a corresponding check (or at least warning) for this now?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(at reset time, not init time)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, the call on curriculum to get a configuration get_config(self, lesson=None) does not have the environment, or the reset_parameters as input. The curriculum does not have a way to check. I would put the check in FloatPropertiesSideChannel, but then I do not have a way to know if reset has been called on the environment yet.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OK. Doesn't have to be in this PR, but I think we should consider the Academy declaring the properties it knows about, and complain (either on python or C#) if it gets a property it doesn't recognize.

Comment thread ml-agents/mlagents/trainers/tests/test_simple_rl.py
Comment thread docs/Migrating.md Outdated
* `CustomResetParameters` are now removed.
* `reset()` on the Low-Level Python API no longer takes a `train_mode` argument. To modify the performance/speed of the engine, you must use an `EngineConfigurationChannel`
* `reset()` on the Low-Level Python API no longer takes a `config` argument. `UnityEnvironment` no longer has a `reset_parameters` field. To modify float properties in the environment, you must use a `FloatPropertiesChannel`. For more information, refer to the [Low Level Python API documentation](Python-API.md)
* The Academy no longer has a `Training Configuration` nor `Inference Configuration` field in the inspector. To modify the configuration form the Low-Level Python API, use an `EngineConfigurationChannel`. To modify it during training, use the new command line arguments `--width`, `--height`, `--quality-level`, `--time-scale` and `--target-frame-rate` in `mlagents-learn`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: "To modify the configuration form from the Low-Level Python API"

Comment thread docs/Python-API.md Outdated

#### EngineConfigurationChannel
An `EngineConfigurationChannel` will allow you to modify how
fast and how accurate the Unity engine is. For example, modifying the
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Not sure accurate is the correct word here. Maybe

An EngineConfiguration will allow you to modify the time scale and graphics quality of the Unity engine.

Perhaps the second sentence is redundant.

@vincentpierre vincentpierre merged commit fa7d381 into develop Dec 4, 2019
@delete-merged-branch delete-merged-branch Bot deleted the develop-side-channel-2 branch December 4, 2019 00:40
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators May 17, 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.

3 participants