Removing Obsolete methods from the package #5024
Conversation
| #pragma warning disable 618 | ||
| return new[] { CreateActuator() }; | ||
| #pragma warning restore 618 | ||
| return new IActuator[0]; |
There was a problem hiding this comment.
This should be abstract (with no implementation).
|
(and changelog) |
| /// </value> | ||
| [Obsolete("VectorActionSize has been deprecated, please use ActionSpec instead.")] | ||
| [FormerlySerializedAs("vectorActionSize")] | ||
| public int[] VectorActionSize = new[] { 1 }; |
There was a problem hiding this comment.
Unfortunately, I think we need to keep these around so that old assets can still be loaded.
There was a problem hiding this comment.
What do you mean by old assets ? What functionality are we trying to preserve ?
There was a problem hiding this comment.
To be clear, for v2, I was thinking refactoring BrainParameters and BehaviorParameters completely, so maybe it is best to set expectations early and tune down the scope I had in mind.
There was a problem hiding this comment.
If you have a scene or prefab from v1.6.0 or earlier, the BrainParameters have VectorActionSize and VectorActionSpaceType fields on them. When you first open this in 1.7.0 or later, we have to read the values from the old field and fill in the ActionSpec (m_ActionSpec) with them. The work happens in the UpdateToActionSpec() method, which is called in OnBeforeSerialize() and OnAfterDeserialize().
So if we remove these and a user loads a scene from <=1.6.0, their Agent will have no actions. In my opinion, this is a big pain for upgrading and we shouldn't do it. I'm willing to be convinced otherwise, but I think we need a bigger discussion on it.
That being said, I don't love that we have to keep the fields around (and public!) just so that they can be read once. It would be worth following up with #devs-serialization to see if there's a better way we can do this while hiding the fields. If nothing else, we can rename them to e.g. DeprecatedDoNotUseVectorActionSize.
There was a problem hiding this comment.
Also, these fields are used in a bunch of places that you didn't update, so this doesn't compile. I'll update yamato tests to run on PRs targeting the staging branch.
| /// </summary> | ||
| [Obsolete("VectorActionSpaceType has been deprecated, please use ActionSpec instead.")] | ||
| [FormerlySerializedAs("vectorActionSpaceType")] | ||
| public SpaceType VectorActionSpaceType = SpaceType.Discrete; |
| @@ -1310,24 +1271,6 @@ public virtual void OnActionReceived(ActionBuffers actions) | |||
| // Nothing implemented. | |||
| return; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
OnActionReceived should be empty now (no reason to look at the actionSpec)
|
It's OK to remove BrainParameters.NumActions ml-agents/com.unity.ml-agents/Runtime/Policies/BrainParameters.cs Lines 106 to 107 in 27f7156 |
…ented Heuristic, Removing NumAction from Brain Parameters
com.unity.ml-agents/CHANGELOG.md
Outdated
| @@ -6,6 +6,21 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) | |||
| and this project adheres to | |||
| [Semantic Versioning](http://semver.org/spec/v2.0.0.html). | |||
|
|
|||
| ## [2.0.0-preview] | |||
There was a problem hiding this comment.
Package verification will probably complain about this. You might need to mark the old Unreleased section as 1.9.0, and this as Unreleased
| /// </summary> | ||
| /// <param name="actionMasker"></param> | ||
| [Obsolete("CollectDiscreteActionMasks has been deprecated, please use WriteDiscreteActionMask.")] | ||
| public virtual void CollectDiscreteActionMasks(DiscreteActionMasker actionMasker) |
There was a problem hiding this comment.
Looks like there's still a reference to CollectDiscreteActionMasks
in docs/Learning-Environment-Design-Agents.md
* Removing Obsolete methods from the package * Missing depecration and modified changelog * Readding the obsolete BrainParameter methods, will need a larger discussion on these * Removing Action Masker, readding the warining when using a non-implemented Heuristic, Removing NumAction from Brain Parameters * removing documentation and some calls to deprecated methods in the extensions package * Editing the Changelog to put the unreleased on top
Proposed change(s)
Removing all methods marked with Obsolete attribute
Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)
Types of change(s)
Checklist
Other comments