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

[Question] Multiple Callbacks For callback_on_new_best #1791

Open
4 tasks done
EitanGronich opened this issue Dec 20, 2023 · 4 comments · May be fixed by #1939
Open
4 tasks done

[Question] Multiple Callbacks For callback_on_new_best #1791

EitanGronich opened this issue Dec 20, 2023 · 4 comments · May be fixed by #1939
Labels
bug Something isn't working help wanted Help from contributors is welcomed question Further information is requested

Comments

@EitanGronich
Copy link

EitanGronich commented Dec 20, 2023

❓ Question

I have a simple use case that I have found no answer to in the documentation. (v. 2.2.1)
I want to pass two chained callbacks as a callback_on_new_best for EvalCallback. (A custom one for drawing a figure and a library one - StopTrainingOnRewardThreshold)
Passing a CallbackList doesn't seem to work, since the parent attribute of the callbacks is None, and the code throws an AssertionError because it asserts their parent is the EvalCallback.
There seem to be no other options except building a custom callback that does them both, but this doesn't seem like good practice given that CallbackList does exist.
Am I missing something?
Minimal reproducible:

import gymnasium as gym
from stable_baselines3 import PPO
from stable_baselines3.common.callbacks import BaseCallback, EvalCallback, StopTrainingOnRewardThreshold, CallbackList

class CustomCallback(BaseCallback):

    def __init__(self, verbose=0):
        super().__init__(verbose)
    
    def _on_step(self):
        # Draw a figure
        return True


eval_env = gym.make("Pendulum-v1")

stop_on_threshold_callback = StopTrainingOnRewardThreshold(reward_threshold=-200, verbose=1)

eval_callback = EvalCallback(eval_env, best_model_save_path="./logs/",
                             log_path="./logs/", eval_freq=500,
                             deterministic=True, render=False,
                             callback_on_new_best=CallbackList([CustomCallback(), stop_on_threshold_callback])
                             )

model = PPO("MlpPolicy", "Pendulum-v1")
model.learn(5000, callback=eval_callback)

Thanks a lot!

Checklist

@EitanGronich EitanGronich added the question Further information is requested label Dec 20, 2023
@araffin araffin added the bug Something isn't working label Jan 12, 2024
@araffin
Copy link
Member

araffin commented Jan 12, 2024

Hello,
thanks for reporting the issue, I think we need to give access to the common parent when using a CallbackList.

@araffin araffin added the help wanted Help from contributors is welcomed label Jan 12, 2024
@icheered
Copy link

Running into the same issue! Is there really no work around?

@araffin
Copy link
Member

araffin commented Jan 15, 2024

Is there really no work around?

a quick workaround is to redefine StopTrainingOnRewardThreshold, combine the two callbacks in one

a better workaround is to fix the code and I would appreciate to receive a PR that solves this issue ;)

@will-maclean
Copy link

I'm happy to work on implementing this one

will-maclean added a commit to will-maclean/stable-baselines3 that referenced this issue May 27, 2024
@will-maclean will-maclean linked a pull request May 27, 2024 that will close this issue
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Help from contributors is welcomed question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants