Skip to content

Conversation

@chriselion
Copy link
Contributor

@chriselion chriselion commented May 21, 2020

Proposed change(s)

Allow enums with ObservableAttribute. Since the number of possibilities is known ahead of time, we can treat this as a one-hot encoding.

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

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

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

}

if (!s_TypeToSensorInfo.ContainsKey(memberType))
if (!s_TypeToSensorInfo.ContainsKey(memberType) && !memberType.IsEnum)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another option would be to add enum types to s_TypeToSensorInfo as they're requested. That simplifies the logic a bit later.

Copy link
Contributor

@vincentpierre vincentpierre left a comment

Choose a reason for hiding this comment

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

I have a minor comment but looks good to me

i++;
}

writer[i] = knownValue ? 0.0f : 1.0f;
Copy link
Contributor

Choose a reason for hiding this comment

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

This means that the dimension of an enum will be n + 1 where n is the number of possible values.
This might be confusing to users and I think returning an array of zeros when the value was none of the possible ones is fair.

Copy link
Contributor

@surfnerd surfnerd left a comment

Choose a reason for hiding this comment

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

agree with @vincentpierre's comment. It seems intuitive to keep the observation all 0's if there was no known enum value selected. Otherwise, looks good.

@chriselion
Copy link
Contributor Author

I made those changes, and also figured out how to support Flags. Non-flags will use all zeros for unknown values. Flags will OR the value with each defined flag.

@chriselion chriselion merged commit ff61216 into master May 26, 2020
@delete-merged-branch delete-merged-branch bot deleted the MLA-1007-enum-observable branch May 26, 2020 22:52
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 27, 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.

4 participants