Returning an `AbstractSpace` object using `get_actions`

Sid-Bhatia-0 commented 4 years ago

get_actions returns some sort of a "set" of all possible actions (both legal and illegal). This seems very much like the idea of returning a Space of all possible actions.

I was wondering if get_actions could return a Space object directly, as in, an instance of a subtype of AbstractSpace.

For discrete actions, presently we are fine returning just a collection of actions using get_actions. But for continuous actions, we won't be able to return something that enumerates over all continous actions and will need to return something like range of real numbers (or whatever is suitable in that context). Again, this is similar to the concept of returning an instance of a subtype of AbstractSpace.

This will also allow us to categorize environments based on a useful taxonomy for environments (derived from the types of their action spaces). One (far-fetched) use-case that comes to my mind is some sort of automated dispatch on a suitable type of algorithm depending upon the type of environment. And algorithmic techniques ofter depend upon the kinds of action spaces (discrete vs. continous, for example).

findmyway commented 4 years ago

Originally, get_actions is required to return an AbstractSpace by default (Actually we can not do this kind of check in Julia). Then I realized that it is too verbose to return a DiscreteSpace for discrete actions. So the restriction is removed.

One (far-fetched) use-case that comes to my mind is some sort of automated dispatch on a suitable type of algorithm depending upon the type of environment.

This is not a big concern. Currently we can safely assume the action space is discrete by default and for other types of action spaces we need to implement the corresponding dispatches (like ContinuousSpace, VectSpace...).

Sid-Bhatia-0 commented 4 years ago

Originally, get_actions is required to return an AbstractSpace by default (Actually we can not do this kind of check in Julia).

What kind of check are you talking about? Do you mean checking whether the return type is an AbstractSpace or not?

for other types of action spaces we need to implement the corresponding dispatches (like ContinuousSpace, VectSpace...).

Are you suggesting adding another trait like ActionSpaceStyle or something?

findmyway commented 4 years ago

Originally, get_actions is required to return an AbstractSpace by default (Actually we can not do this kind of check in Julia).

What kind of check are you talking about? Do you mean checking whether the return type is an AbstractSpace or not?

Yes

for other types of action spaces we need to implement the corresponding dispatches (like ContinuousSpace, VectSpace...).

Are you suggesting adding another trait like ActionSpaceStyle or something?

No, I mean even though get_actions doesn't always return an AbstractSpace, we can still safely handle this case by treating the return as discrete actions by default.

Sid-Bhatia-0 commented 4 years ago

Originally, get_actions is required to return an AbstractSpace by default (Actually we can not do this kind of check in Julia).

Can we not use something like the following?

function get_actions(...)::AbstractSpace
...
end

findmyway commented 4 years ago

Yes of course. But users can still define a function with a return type other than AbstractSpace, right?

https://stackoverflow.com/questions/34147132/parametric-return-types

Sid-Bhatia-0 commented 4 years ago

But users can still define a function with a return type other than AbstractSpace, right?

Oh, okay... I was thinking of enforcing the return type to be a subtype of AbstractSpace as a design choice so as to bring consistency among all environments in the JuliaRL ecosystem. We would have most of the common spaces covered and if users they want something customized, they can always define their own subtypes of AbstractSpace if necessary.

So right now, is there a way for us to extract information about the action space of an environment being continuous vs. discrete, for example?

findmyway commented 4 years ago

Yes. Like I said above, we can safely assume get_actions returns a discrete action space. In other cases, you dispatch on the specific space you want.

Sid-Bhatia-0 commented 4 years ago

I'm sorry for asking this again, but I am still confused.

Yes. Like I said above, we can safely assume get_actions returns a discrete action space.

You said that we can safely assume get_actions returns a discrete action space. Cool. This means that we don't have the type information (in terms of DiscreteSpace, ContinuousSpace etc.) about whatever was returned by get_actions. Let us say the user returns a Vector of length 2. Then we assume by default that the elements of this Vector represents actions in a discrete action space. But what if these two numbers are meant to represent the lower and upper bounds of continous action space, for example? (By the way, is this related to type-piracy?)

In other cases, you dispatch on the specific space you want.

When you say dispatch, what are you suggesting to dispatch on? The return type of get_actions? But not all users are using a starndard set of get_action return types (at least right now) for their environments. If we don't enforce consistency by the use of a standardized heirarchy under the AbstractSpace, User1 could define their own ContinousSpace1 type to be returned for their environment and User2 might defined ContinousSpace2 for their environment, even though they both might serve the same purpose. What I am saying that if they are using the same functionality, then we can prevent this from happening by enforcing consistency with the use of ContinousSpace. And in case if they need something special for their case that is not already provided by us, users can always create their own Space subtypes of AbstractSpace, but in general, we should try to reuse as much of the library code as possible.

If these types are defined completely independently by each user for their own needs, one problem I see is a lot of redundant code if they need the same functionality. Another thing which I did not understand is how would you write generic code that dispatches on ContinousSpace if each user defines their own version of ContinousSpace to be returned (since we are not standardizing it ourselves).

findmyway commented 4 years ago

Another thing which I did not understand is how would you write generic code that dispatches on ContinousSpace if each user defines their own version of ContinousSpace to be returned (since we are not standardizing it ourselves).

The idea is simple. We convert the user's own action space into a known space:

https://github.com/JuliaReinforcementLearning/ReinforcementLearningBase.jl/blob/f2d4f5df04801495fe4c970f9b9ad50c7a5263bd/src/implementations/spaces/discrete_space.jl#L59-L62

But what if these two numbers are meant to represent the lower and upper bounds of continous action space, for example?

No, this is ambiguous here. One should return a ContinuousSpace in this case.

I understand your concern here. But we need to weigh the benefits of allowing flexibility against introducing too much restrictions.

One reason I prefer to allowing more flexible return types of get_actions is to move out all the AbstractSpace related concepts. But I haven't got enough time to think about it in depth. So things are just kept as it is now.

Sid-Bhatia-0 commented 4 years ago

Thanks a lot! I have a better understanding of it now.

One reason I prefer to allowing more flexible return types of get_actions is to move out all the AbstractSpace related concepts.

So are you suggesting something like moving it all out into a Spaces package or something? And then using Spaces in RLBase? This sounds good. After all, the concept of Spaces is something that can be useful outside of RLBase as well. By the way, I suppose there isn't any package that already does this kind of thing?

The idea is simple. We convert the user's own action space into a known space

Okay. So from what I understood so far, there are two ways to go about this whole business. One is to use conversion, as you suggested. Like we move out all the AbstractSpace related concepts into a Spaces package and then people can write converters from their custom space types to the ones provided by the Spaces package if needed. The other way perhaps what I was suggesting, which was to always subtype AbstractSpace in case a user needs some customization.

So now, if we separate out a Spaces package, then users have both choices. They can either write their own converters to convert their custom types to some standard space provided by Spaces, or they can choose to subtype AbstractSpace for custom needs.

findmyway commented 4 years ago