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

Fix bug in retro_branching when done on reset #4

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dmitrySorokin
Copy link

@cwfparsonson
Copy link
Owner

Hi @dmitrySorokin

Many thanks for the PR!

I am reluctant to merge your changes in case they cause deviations from the original paper's experiments, which I believe should run.

Did the error with the retro branch in reset occur when you tried to run the paper's experiments, or when you tried to use retro branching in your own code? If the latter, I would recommend that you use the 'cleaner' version of retro branching in the notebook tutorial, which you should just be able to copy-paste (with the SearchTree class from the other tutorial) into your own project. These tutorials are in notebooks/tutorials/

@dmitrySorokin
Copy link
Author

Hi @cwfparsonson

Thanks for the answer!
I will take a look on the notebooks.
The error appeared when I run new experiments with your code.

@cwfparsonson
Copy link
Owner

Also notice that in the notebook example, we call extract() on the retro branching reward object before the environment loop starts (ie just after we reset the environment) - I suppose this should really be a separate reset() method rather than extract(), but you should extract() before starting the env loop. Would this fix the error you were finding with extract()?

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.

2 participants