BehaviorParameters being Internal breaks code and makes automation impossible

[MikeWise2718]

Making the BehaviorParameters internal, and all its fields internal breaks my code, since I like to build agents with code. This means I add the components with script and set the parameters that way too. This feels to me like a more scalable and testable approach. Unless I am overseeing something (which is entirely possible), this means I now have to base everything on hand constructed prefabs.

Isn't this a pretty severe limitation? How do you write your own test cases against it?

I would think that anything exposed in a Unity Inspector should be public, since someone might want to modify it or inspect it programmatically. I think most everything in this inspector is now inaccessable:

[chriselion]

Hi @MikeWise2718,
Thanks for raising this issue.

Just a little background: we've been trying to reduce the surface area of our public API as we get ready to become a preview package. Once we hit that milestone, we have to follow strict semantic versioning on the API, and any breaking change (like renaming a variable or method) requires a major version increase. Unfortunately, there's no way to mark some parts of the API as "stable" and others "experimental", so we tend to be defensive and don't expose things we're not 100% sure about.

That being said, I agree with you that we went too far on this by making BehaviorParameters internal (and probably BrainParameters too). We'll work on revising this in the next few days (probably in the form of private members and public properties).

How do you write your own test cases against it?

All of unit tests have access to internal classes/members due to the InternalsVisibleTo statements in our AssemblyInfo.cs:

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

Lines 3 to 4 in 2c2f930

	[assembly: InternalsVisibleTo("Unity.ML-Agents.Editor.Tests")]
	[assembly: InternalsVisibleTo("Unity.ML-Agents.Editor")]

But this is definitely discouraging us from eating our own dogfood, so we're going to add some additional test cases without this access, to make sure that operations like creating Agents and attaching custom sensors are possible for users to do via code.

I would think that anything exposed in a Unity Inspector should be public, since someone might want to modify it or inspect it programmatically. I think most everything in this inspector is now inaccessable.

I agree in general, but there are a few details that need more thought/work on our side:

• Fields that affect the model (e.g. observation and action sizes in BehaviorParameters) can't be changed once simulation starts. Therefore we should probably disable them in the Inspector when in Play mode.
• Adding a get property for the fields is straightforward, but having a public set is complicated - if you try to change the observation size after simulation has started, we'd probably have to ignore the new value and fire a warning.
• Some fields are only read at initialization time (e.g. Behavior Type in BehaviorParameters), so it takes a bit of extra work to make sure these changed propagate when they're changed in the Inspecter or from code.

I'm not saying the extra effort is a reason not to do it :) Just that it might take a few more days to get it fixed on master.

I'm open to other suggestions on how to keep this user-friendly without letting you shoot yourself in the foot...

[MikeWise2718]

Great, I look forward to running my example against it and seeing if I can get it to work again. Let me know if I can help test it in anyway.

[JohnBergago]

Hi @chriselion,
I came across this issue and would like to mention, that I have the same problem for several sensor components such as RayPerception and CameraSensor. I really like to create and configure them on start-up. I'm using side-channels to specify the settings and configuration from python code. That way it is easy to have multiple experiments running in parallel without having a slightly different environment for each experiment.
Until recently I could simply add the sensors I needed in InitializeAgent() before the sensors were created. However, this is not possible anymore as properties such as height and width in camera sensors or raysPerDirection, detectableTags in RayPerceptionSensors are only internally settable.

I get your point, that it gets complicated with the setters after simulation started, but what about some kind of initialization method, that will only have effect before the Sensor component was created. Otherwise it would fire a warning. That way nothing will be changed during simulation, but preserve the freedom to configure everything from code.

[chriselion]

Hi @JohnBergago,
Thanks for the feedback. Converting the internal setters to public is on my list of things to do this week.

[chriselion]

Oops, didn't mean to close this.

[MikeWise2718]

Finally got back to this (external events are intervening), and I got a lot further, but now it seems I need to add a "DecisionRequester" to my agent, and that is an internal type. So I am not quite able to automate my agent creation yet. Without that it just sits there...

[chriselion]

DecisionRequester is on our list of things to review too, but we missed the cutoff for the last release.

DecisionRequester just calls Agent.RequestDecision() at a regular frequency; this is public and you can call it on the steps you want your Agent to observe and act.

[chriselion]

Sorry, I realized I never followed up on this. DecisionRequester was made public in #3716

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.