[Side Channels] Edited the documenation and renamed a few things #3833
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mmattar
reviewed
Apr 23, 2020
com.unity.ml-agents/CHANGELOG.md
Outdated
| - The SideChannel API has changed: | ||
| - `EnvironmentParameters` replaces the default `FloatPropertiesChannel`. You can access the `EnvironmentParameters` with `Academy.Instance.EnvironmentParameters` on C# and create an `EnvironmentParametersChannel` on Python | ||
| - `SideChannelUtils` was renamed `SideChannelManager` | ||
| - The `Academy` instance now has a `StatsRecorder` property |
There was a problem hiding this comment.
Do we want to emphasize that the StatsSideChannel is now internal and that this is the new way
There was a problem hiding this comment.
Also mention that EngineConfigurationChannel is now internal?
There was a problem hiding this comment.
Worth mentioning that OnMessageReceived is now internal?
There was a problem hiding this comment.
StatsSideChannel hasn't actually been released yet, so we should just edit the existing line for it.
docs/Migrating.md
Outdated
| @@ -33,6 +33,11 @@ double-check that the versions are in the same. The versions can be found in | |||
| - The signature of `Agent.Heuristic()` was changed to take a `float[]` as a | |||
| parameter, instead of returning the array. This was done to prevent a common | |||
| source of error where users would return arrays of the wrong size. | |||
| - The SideChannel API has changed: | |||
There was a problem hiding this comment.
We need to mention the user-facing changes of going from StatsSideChannel to Academy.Instance.StatsRecorder
There was a problem hiding this comment.
And that StatsSideChannel is now internal
docs/Migrating.md
Outdated
| @@ -33,6 +33,11 @@ double-check that the versions are in the same. The versions can be found in | |||
| - The signature of `Agent.Heuristic()` was changed to take a `float[]` as a | |||
| parameter, instead of returning the array. This was done to prevent a common | |||
| source of error where users would return arrays of the wrong size. | |||
| - The SideChannel API has changed: | |||
There was a problem hiding this comment.
Should we add that EngineConfigurationChannel is no internal
There was a problem hiding this comment.
Worth mentioning that OnMessageReceived is now internal?
docs/Migrating.md
Outdated
| - `EnvironmentParameters` replaces the default `FloatPropertiesChannel`. You can access the `EnvironmentParameters` with `Academy.Instance.EnvironmentParameters` | ||
| - `SideChannelUtils` was renamed `SideChannelManager` | ||
| - The `Academy` instance now has a `StatsRecorder` property | ||
| - `SideChannelManager.GetSideChannel(s)` has been removed from the API |
chriselion
reviewed
Apr 23, 2020
com.unity.ml-agents/CHANGELOG.md
Outdated
| - `EnvironmentParameters` replaces the default `FloatPropertiesChannel`. You can access the `EnvironmentParameters` with `Academy.Instance.EnvironmentParameters` on C# and create an `EnvironmentParametersChannel` on Python | ||
| - `SideChannelUtils` was renamed `SideChannelManager` | ||
| - The `Academy` instance now has a `StatsRecorder` property | ||
| - `SideChannelManager.GetSideChannel(s)` has been removed from the API |
There was a problem hiding this comment.
Same - SideChannelManager/Utils wasn't in 0.15, so we don't need to mention it here.
chriselion
reviewed
Apr 23, 2020
| @@ -54,6 +59,10 @@ double-check that the versions are in the same. The versions can be found in | |||
| - If your Agent class overrides `Heuristic()`, change the signature to | |||
| `public override void Heuristic(float[] actionsOut)` and assign values to | |||
| `actionsOut` instead of returning an array. | |||
| - If you used `SideChannels` you must: | |||
There was a problem hiding this comment.
Revise the "Replace Academy.FloatProperties..." lines above instead.
chriselion
suggested changes
Apr 23, 2020
chriselion
reviewed
Apr 23, 2020
Co-Authored-By: Chris Elion <[email protected]>
chriselion
reviewed
Apr 23, 2020
Co-Authored-By: Chris Elion <[email protected]>
vincentpierre
added a commit
that referenced
this pull request
Apr 23, 2020
* Make EnvironmentParameters a first-class citizen in the API Missing: Python conterparts and testing. * Minor comment fix to Engine Parameters * A second minor fix. * Make EngineConfigChannel Internal and add a singleton/sealed accessor * Make StatsSideChannel Internal and add a singleton/sealed accessor * Changes to SideChannelUtils - Disallow two sidechannels of the same type to be added - Remove GetSideChannels that return a list as that is now unnecessary - Make most methods except (register/unregister) internal to limit users impacting the “system-level” side channels - Add an improved comment to SideChannel.cs * Added Dispose methods to system-level sidechannel wrappers - Specifically to StatsRecorder, EnvironmentParameters and EngineParameters. - Updated Academy.Dispose to take advantage of these. - Updated Editor tests to cover all three “system-level” side channels. Kudos to Unit Tests (TestAcademy / TestAcademyDispose) for catching these. * Removed debub log. * Back-up commit. * Revert "Back-up commit." This reverts commit f81e835. * key changes to wrapper classes made the wrapper classes non-singleton (but internal constructors) made EngineParameters internal * Re-enabled the option to add multiple side channels of the same type * Fixed example env * Add an enum flag to the EnvParamsChannel * Adding .cs.meta files * Update engine config side channel Removed unnecessary accessors Made capture frame rate a parameter * Rename SideChannelUtils —> SideChannelsManager * PR feedback * Minor PR feedback. * Python side changes to the SideChannel redesign (#3826) * Modified the EngineConfig to send one message per field * Created the Python Environment Parameters Channel and hooked it in * Make OnMessageReceived protected * addressing comments * [Side Channels] Edited the documenation and renamed a few things (#3833) * Edited the documetation and renamed a few things * addressing comments * Update docs/Python-API.md Co-Authored-By: Chris Elion <[email protected]> * Update com.unity.ml-agents/CHANGELOG.md Co-Authored-By: Chris Elion <[email protected]> * Removing unecessary migrating line Co-authored-by: Chris Elion <[email protected]> * Addressing renaming comments * Removing the EngineParameters class Co-authored-by: Vincent-Pierre BERGES <[email protected]> Co-authored-by: Chris Elion <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed change(s)
Update to the documentation for #3807
Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)
Types of change(s)
Checklist
Other comments