search

FluxML / Gym.jl

Gym environments in Julia
MIT License
54 stars 19 forks source link

Travis for the Gym #14

Open kraftpunk97-zz opened 5 years ago

kraftpunk97-zz commented 5 years ago

Can we get travis for the gym?

v-i-s-h commented 5 years ago

By travis, I think you meant to start the CI testing processes. I will be happy to contribute to this part. Can we discuss a timeline and things to include? Maybe this is also the time we should think about starting the documentation process.

tejank10 commented 5 years ago

Yes, we should now start documentation. As far as travis CI is concerned, we need to build some tests. For eg, test to check if every action gives expected behaviour: Given state St , we get correct next state S(t+1). This can be tested for every action for an environment with discrete actions. It is possible, at least now, given that Action space is small. For those with continuous action space, we can choose certain values in the action space and write tests.

Some more things which would be good to see in future are having a registry #10 .

Thoughts on adding more tests are welcome.

kraftpunk97-zz commented 5 years ago

@tejank10 I have certain tests in the Spaces directory. I picked them up from the openai/gym but only the ones that seemed relevant to what this package offers. Why don't we start from there and then build off of that?

Registry is of course next on the to-do list. I have given it a lot of thought, and it should be up and running within a few days.

v-i-s-h commented 5 years ago

Hi, I'm planning to work on the test part. I wrote some test for Box spaces and is available at my fork https://github.com/v-i-s-h/Gym.jl/blob/dev-vish/test/Spaces/box.jl. It will be great if you people can give me some early comments.

tejank10 commented 5 years ago

Looks good to me. @kraftpunk97 thoughts?

kraftpunk97-zz commented 5 years ago

@v-i-s-h This set of test is very exhaustive. Great. I think we can add one more test to this, in which low values are actually greater than the high values of the Box. The python gym's version of the Box space can handle this scenario.

v-i-s-h commented 5 years ago

@tejank10 @kraftpunk97 Thanks for the comments. I will add more tests and open a PR in some days.

One general workflow doubt: To get the "low<high" updates from @kraftpunk97, I added kraftpunk97/Gym.jl as one of my remotes. In my fork also, I have branch to work for tests. Now if I merge kraftpunk97's updates to my test branch and add my test to that branch, will it create a conflict to merge in main repo later when I try a PR with both kraftpunk's updates and my tests?

tejank10 commented 5 years ago

Shouldn't be a problem I suppose. I have merged @kraftpunk97 's PR now.