AlgorithmConfig.to_dict() returns misleading values for new-API-stack

[corentinlger]

Hello, when creating an algorithm config with the new RLlib API stack, config.to_dict()["train_batch_size"] returns the stale class default (4000 for PPO) instead of the value I set.

e.g if I set setting train_batch_size_per_learner=10 and only use 1 learner, I would expect to read back 10 from this field rather than 4000.

Question:

This is not a major problem as you can still always access the true batch size value through the config, so first I was wondering if to_dict() shouldn't be used at all on with the new API ? But either way the current behavior can be a bit misleading.

So in to_dict(), when enable_rl_module_and_learner is True, would it make sense to overwrite config["train_batch_size"] with self.total_train_batch_size (or any other field that has the same "issue") ? Or maybe to document this in the to_dict() docstring ?

Reproduction:

from ray.rllib.algorithms.ppo.ppo import PPOConfig

config = PPOConfig().training(
    train_batch_size_per_learner=10,
    minibatch_size=5,
)

print(config.total_train_batch_size)
print(config.train_batch_size_per_learner)
print(f"{config.to_dict()['train_batch_size']} - missleading field ") # missleading field
print(config.to_dict()["_train_batch_size_per_learner"])
print(config.to_dict().get("total_train_batch_size", "Not found"))
print(config.to_dict().get("train_batch_size_per_learner", "Not found"))

10
10
4000 - missleading field 
10
Not found
Not found

Thanks in advance !

[AyushKashyapII]

Hey @corentinlger, this is a great catch! You are exactly right about the cause: to_dict() uses vars() which only dumps the raw internal attributes (like the stale 4000 default) and completely misses the dynamically calculated @Property getters of the new API stack.
I've gone ahead and written a fix for this. I patched to_dict() so that when enable_rl_module_and_learner=True, it explicitly overwrites the legacy train_batch_size with self.total_train_batch_size, and also exposes total_train_batch_size and train_batch_size_per_learner so they no longer return 'Not found'.
Opening a PR for this right now!