Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove {text,custom} {action,observations} #2839

Merged
merged 9 commits into from
Nov 4, 2019

Conversation

chriselion
Copy link
Contributor

@chriselion chriselion commented Nov 1, 2019

Removes text actions and observations, and custom action and observation protos. These were not integrated into training (i.e. there was no way to feed text observations into the trainer, or produce custom actions).

@surfnerd
Copy link
Contributor

surfnerd commented Nov 4, 2019

looks like the Yamato triggers weren't picked up. I was assuming the develop- branch name style. Is that more of a soft rule? Should the trigger just be for develop as a target of a PR?

@surfnerd
Copy link
Contributor

surfnerd commented Nov 4, 2019

Otherwise, this is a thing of beauty, thank you.

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.

🌆 🇮🇹

@chriselion chriselion merged commit 50c8ca5 into develop Nov 4, 2019
@delete-merged-branch delete-merged-branch bot deleted the remove-text-custom-action-obs branch November 4, 2019 20:29
@GProulx
Copy link
Contributor

GProulx commented Nov 19, 2019

Hi @chriselion

I’m really surprised to discover that the project is getting rid of CustomObservation. For me CustomObs is very helpful. For example to return data of variable length on each Step or based on some initials reset parameters. Using ProtoBuf message is also interesting to easily manipulate those received objects in Python directly.

I already thought about a few options to work around that suppression but can you tell me why that decision was made and if you have some alternative solutions to suggest?

Thanks

@chriselion
Copy link
Contributor Author

Hi @GProulx,
We felt there were a couple of problems with the old approach. First, modifying the custom proto definitions and rebuilding the code was a source of problems for some users, especially on Windows.
The larger problem was that just defining the custom observations and actions wasn't enough for the trainers to be able to do anything with them; there was an expectation by some users that all they would need to do is define the custom protos, and suddenly the trainer would start consuming the observations or producing the actions, whereas it would actually require a lot of customization to enable this, and we had no good examples of this.

The recent ISensor interface changes give us a chance to bring some of this back in a different way. I think we can add a new "compression type" to transmit a Google.Protobuf.WellKnownTypes.Any, and provide a corresponding user-defined hook in the python side to decode this into an observation.

Do you have more details on how you were using the Custom Observations and feeding them into the trainer?

@dlindmark
Copy link

I am working on a project that planned on using custom observations as well. We want to use visual observations, but not in an end-to-end fashion. But rather using seperate computer vision algorithms together with semantic segmentation networks to "interept" the images. The result is then used as an vector observation. This way we can train the policy both with the true observation that is easy to know from the simulation. But also using a seperate module that estimates these observations from an image.

So we have implemented so that we can send images using the custom observation proto. And in brain.py we process these images and changes the values on some of the vector observations. We could of course do this processing already in the CollectObservations() function in unity. But our toolchain makes it easier to do that step in python.

@dlindmark
Copy link

The recent ISensor interface changes give us a chance to bring some of this back in a different way. I think we can add a new "compression type" to transmit a Google.Protobuf.WellKnownTypes.Any, and provide a corresponding user-defined hook in the python side to decode this into an observation.

Is there any current work on bringing the custom observation back in the form you described? Or an issue for it?

@chriselion
Copy link
Contributor Author

Sorry for the delay. We have this logged as MLA-348 in our tracker, but it's not currently scheduled to be worked on.

The current master branch also has a new feature called Side Channels which can be used to send arbitrary data between the python and C# processes


@GProulx
Copy link
Contributor

GProulx commented Mar 26, 2020

@chriselion , just a very very! late update on that topic to mention you that the new Side Channel replacement of the CustomObservation works even better that the latter for us! Thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 15, 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.

5 participants