-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
Added the fundamental spaces #7
Conversation
…into Base-structure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kraftpunk97 for great work!
src/Spaces/discrete.jl
Outdated
Discrete(N::Int) = new(N, Int64) | ||
end | ||
|
||
sample(self::Discrete) = rand(1:self.n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self
-> d
(or anything else which would imply that given object is a Discrete
when read)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tejank10 A little confused here. Are you suggesting something like sample(discrete_obj::Discrete) = rand(1:discrete_obj.n)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, something like that.
as_int = Int.(x) | ||
catch InexactError | ||
return false | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clever use of try catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌👍
src/Spaces/multi-binary.jl
Outdated
|
||
sample(self::MultiBinary) = (self.dtype).(rand(0:1, self.n)) | ||
|
||
contains(self::MultiBinary, x) = all(boolor.(x .== 0, x .== 1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any particular reason for introducing boolor
? Can we do all(x .== 0 .| x .== 1)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about that, I was just testing something and I forgot to remove that function. Consider it fixed...
src/Spaces/multi-binary.jl
Outdated
|
||
contains(self::MultiBinary, x) = all(boolor.(x .== 0, x .== 1)) | ||
|
||
Base.:(==)(self::MultiBinary, other::MultiBinary) = return self.n == other.n |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return
can be dropped here
@tejank10 Made changes as requested. Please review and verify. |
Great! I intend to replace CartPole with CartPole1.1, if the examples in model-zoo are not breaking. |
Made the necessary changes. Please verify. |
Great work @kraftpunk97 , Thanks! |
I have never experienced joy like this before. Thanks for sticking it out with me. |
PR with reference to this issue #5
seed and copy functions still need to be implemented. Modified CartPole.jl (CartPole-v1.1.jl) to run on these spaces.