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

Adding Dynamic Obstacle env, I closed the last one because my commits got a bit messy. #46

Merged
merged 13 commits into from
Oct 13, 2020

Conversation

RajGhugare19
Copy link
Contributor

I think this should work now, but tell me if anything is wrong

Copy link
Member

@findmyway findmyway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

How about adding a GIF in readme?

flag = 0
#flag indicated whether
#1)the new-position of the kth obstacle overlaps the new-positions of [1,k-1] obstacles
#2)the new-position of an obstacle is the same as it's old position
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can allow the obstacle to stay the same as its old position now? Considering the extreme case that the last obstacle is surrounded by 8 obstacles (I know is really a corner case...), it will never break out if we don't allow it to be the same with its old position.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll change this tmrow and attach a GIF to the readme as well :)

@codecov-io
Copy link

codecov-io commented Oct 13, 2020

Codecov Report

Merging #46 into master will decrease coverage by 6.38%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #46      +/-   ##
==========================================
- Coverage   37.54%   31.16%   -6.39%     
==========================================
  Files          13       14       +1     
  Lines         293      353      +60     
==========================================
  Hits          110      110              
- Misses        183      243      +60     
Impacted Files Coverage Δ
src/envs/doorkey.jl 0.00% <ø> (ø)
src/envs/dynamicobstacles.jl 0.00% <0.00%> (ø)
src/objects.jl 16.12% <0.00%> (-1.12%) ⬇️
src/envs/collectgems.jl 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d004c7...ddb7a29. Read the comment docs.

@RajGhugare19
Copy link
Contributor Author

@findmyway I have made the final changes and fixed a few errors. 😃

@findmyway
Copy link
Member

Cool! One final minor issue. It seems that in your GIF the obstacle may be placed in the same position of the GOAL. Could you add a check to avoid this case?

@RajGhugare19
Copy link
Contributor Author

@findmyway Ofcourse. Done

@findmyway findmyway merged commit 59bc567 into JuliaReinforcementLearning:master Oct 13, 2020
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

Successfully merging this pull request may close these issues.

None yet

3 participants