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

Travis for the Gym #14

Open
kraftpunk97-zz opened this issue Apr 8, 2019 · 8 comments
Open

Travis for the Gym #14

kraftpunk97-zz opened this issue Apr 8, 2019 · 8 comments

Comments

@kraftpunk97-zz
Copy link
Contributor

Can we get travis for the gym?

@v-i-s-h
Copy link

v-i-s-h commented Apr 9, 2019

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
Copy link
Contributor

tejank10 commented Apr 9, 2019

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
Copy link
Contributor Author

kraftpunk97-zz commented Apr 9, 2019

@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
Copy link

v-i-s-h commented Apr 17, 2019

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
Copy link
Contributor

Looks good to me. @kraftpunk97 thoughts?

@kraftpunk97-zz
Copy link
Contributor Author

kraftpunk97-zz commented Apr 19, 2019

@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
Copy link

v-i-s-h commented Apr 19, 2019

@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
Copy link
Contributor

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

@kraftpunk97-zz kraftpunk97-zz mentioned this issue May 30, 2019
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants