[RLlib] Fix AlgorithmConfig.to_dict() for New API Stack by AyushKashyapII · Pull Request #63695 · ray-project/ray

AyushKashyapII · 2026-05-28T11:51:09Z

Why are these changes needed?

This PR fixes #63669.

Currently, when a user is on the New API Stack (enable_rl_module_and_learner=True) and sets train_batch_size_per_learner, config.to_dict() returns misleading data. Because to_dict() relies on vars(self), it dumps the stale legacy train_batch_size attribute (e.g., 4000) and fails to expose the new calculated properties, leading to incorrect experiment logging.

What this PR does:

Updates AlgorithmConfig.to_dict() to explicitly check if enable_rl_module_and_learner is True.
Dynamically overwrites the legacy train_batch_size key with the true self.total_train_batch_size.
Injects total_train_batch_size and train_batch_size_per_learner into the dictionary so they no longer return as "Not found".
Updates the method docstring to warn users about this dynamic replacement.

Related issue number

Fixes #63669

Signed-off-by: Ayush KAshyap <kashyap11ayush02@gmail.com>

gemini-code-assist

Code Review

This pull request updates AlgorithmConfig.to_dict() to dynamically overwrite the legacy train_batch_size key with total_train_batch_size and explicitly expose new API variables when using the new API stack. However, adding total_train_batch_size to the returned dictionary will cause update_from_dict() to crash with an AttributeError because total_train_batch_size is a read-only property without a setter.

gemini-code-assist · 2026-05-28T11:52:32Z

+            config["train_batch_size"] = self.total_train_batch_size
+
+            # 2. Expose the new API variables explicitly so they don't return "Not found"
+            config["total_train_batch_size"] = self.total_train_batch_size


Adding total_train_batch_size to the dictionary returned by to_dict() will cause update_from_dict() (and consequently from_dict()) to crash with an AttributeError.\n\n### Why this happens:\n1. to_dict() now injects "total_train_batch_size" into the returned dictionary.\n2. When restoring a config or updating it from a dictionary (e.g., during Tune trial creation or checkpoint restoration), update_from_dict() iterates over the dictionary keys and calls setattr(self, key, value) for any keys not explicitly handled.\n3. total_train_batch_size is a read-only property on AlgorithmConfig (it has a getter but no @total_train_batch_size.setter).\n4. In Python, calling setattr() on a read-only property raises an AttributeError: can't set attribute.\n\n### How to fix:\nTo prevent this crash, you must either:\n- Add a dummy setter for total_train_batch_size in AlgorithmConfig (around line 4291) that ignores the value or handles it appropriately:\n python\n @total_train_batch_size.setter\n def total_train_batch_size(self, value: int) -> None:\n pass\n \n- Or, avoid adding total_train_batch_size to the dictionary in to_dict() if it is not strictly required by external legacy APIs.

…e_from_dict crash Signed-off-by: Ayush KAshyap <kashyap11ayush02@gmail.com>

AyushKashyapII · 2026-05-28T12:22:49Z

I thought heavily about adding a dummy setter, but I realized that's an anti-pattern that could lead to silent failures. Instead, I went with the cleaner approach: I omitted the read-only total_train_batch_size key entirely.
The PR now just overwrites the legacy train_batch_size key with the calculated total, and exposes train_batch_size_per_learner. I wrote a local test suite to verify this, and it perfectly solves the original issue while keeping update_from_dict() round-tripping completely happy without crashing!

mock_test.py

…API stack When enable_rl_module_and_learner=True, to_dict() was injecting 'train_batch_size_per_learner' as a computed value into the dictionary. This caused a lossy round-trip through update_from_dict(): the property setter would permanently pin _train_batch_size_per_learner, breaking the dynamic inference link from train_batch_size and num_learners. Fix: only overwrite the legacy 'train_batch_size' key with the computed total_train_batch_size. The private backing field '_train_batch_size_per_learner' (which may be None for inferred configs) is already serialized correctly via vars(self) and round-trips safely. Fixes: ray-project#63669 Signed-off-by: Ayush KAshyap <kashyap11ayush02@gmail.com>

cursor

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{Reviewed by Cursor Bugbot for commit 02f8ce9. Configure here.}

pseudo-rnd-thoughts

Thanks for the PR @AyushKashyapII
Could you add a test to check the roundtripping of to_dict

Signed-off-by: Ayush KAshyap <kashyap11ayush02@gmail.com>

AyushKashyapII · 2026-06-01T11:49:11Z

Thanks for the review!
I've just added test_to_dict_roundtrip_new_api_stack to test_algorithm_config.py. It explicitly verifies that the dictionary correctly outputs train_batch_size based on the formula, successfully round-trips through update_from_dict() without raising an AttributeError on the read-only property, and perfectly preserves the dynamic state of train_batch_size_per_learner.

Let me know if you need any other tests added!

Signed-off-by: Ayush KAshyap <kashyap11ayush02@gmail.com>

pseudo-rnd-thoughts

LGTM

pseudo-rnd-thoughts · 2026-06-02T14:53:16Z

@AyushKashyapII The test is failing with KeyError: 'train_batch_size_per_learner'

Signed-off-by: Ayush KAshyap <kashyap11ayush02@gmail.com>

AyushKashyapII · 2026-06-02T17:06:49Z

Hey @pseudo-rnd-thoughts Thanks for catching that!Just pushed a fix for both issues:

Updated the test assertion–it was incorrectly looking for the public property we just removed from to_dict(), so I swapped it to check the private variable instead.
Fixed a crash in DreamerV3 – algorithms that don't use train_batch_size (where it's None) were crashing during to_dict(), so I updated the properties to gracefully handle None values.
Tests should be green now! Let me know if everything looks good.

pseudo-rnd-thoughts

Test looks good but the if statements don't make sense to me

pseudo-rnd-thoughts · 2026-06-04T11:27:40Z

        # If not set explicitly, try to infer the value.
        if self._train_batch_size_per_learner is None:
+            if self.train_batch_size is None:


I think this should raise an error. When would this occur?

This scenario actually occurs when an algorithm explicitly unsets train_batch_size because it uses a completely different batching logic. For example, DreamerV3 sets train_batch_size = None and uses its own batch_size_B and batch_length_T variables instead. If we raise an error here, calling config.to_dict() on a DreamerV3 config will crash because to_dict() attempts to evaluate total_train_batch_size for everything using the new API stack. However, you are right that for standard algorithms we should fail loudly. I've updated the PR to raise a ValueError here and catch the error inside to_dict() so that algorithms with non-standard batching still work.

pseudo-rnd-thoughts · 2026-06-04T11:27:48Z

        """Returns the effective total train batch size.

        New API stack: `train_batch_size_per_learner` * [effective num Learners].

        @OldAPIStack: User never touches `train_batch_size_per_learner` or
        `num_learners`) -> `train_batch_size`.
        """
+        if self.train_batch_size_per_learner is None:


Signed-off-by: Ayush KAshyap <kashyap11ayush02@gmail.com>

AyushKashyapII · 2026-06-09T13:16:09Z

Hi @pseudo-rnd-thoughts ,

I wanted to follow up on this PR. The review feedback has been incorporated and the PR is currently approved. If the implementation looks good, would it be possible to move it forward for merge? If there are any remaining concerns or changes you'd like to see, I'm happy to address them.

Thanks again for your help and review..

pseudo-rnd-thoughts

Just one last comment

pseudo-rnd-thoughts · 2026-06-12T09:34:18Z

+        if self.enable_rl_module_and_learner:
+            try:
+                config["train_batch_size"] = self.total_train_batch_size
+            except ValueError:


When could this ValueError happen?

its raised by the @Property getters when both _train_batch_size_per_learner and train_batch_size are None or neither batch size has been configured.
This happens when to_dict() is called on a partially-configured AlgorithmConfig before the user has explicitly set any batch size (like maybe during initialization, or serialization before .validate() is called). It also happens for algorithms that intentionally leave standard batch sizes unset (like DreamerV3).
The try/except lets to_dict() handle this gracefully .

AyushKashyapII · 2026-06-18T18:15:10Z

Hi @pseudo-rnd-thoughts , following up on this PR. The review comments have been resolved and approved. Is there anything else needed before it can be merged?

…63695) ## Why are these changes needed? This PR fixes ray-project#63669. Currently, when a user is on the New API Stack (`enable_rl_module_and_learner=True`) and sets `train_batch_size_per_learner`, `config.to_dict()` returns misleading data. Because `to_dict()` relies on `vars(self)`, it dumps the stale legacy `train_batch_size` attribute (e.g., 4000) and fails to expose the new calculated properties, leading to incorrect experiment logging. **What this PR does:** - Updates `AlgorithmConfig.to_dict()` to explicitly check if `enable_rl_module_and_learner` is True. - Dynamically overwrites the legacy `train_batch_size` key with the true `self.total_train_batch_size`. - Injects `total_train_batch_size` and `train_batch_size_per_learner` into the dictionary so they no longer return as "Not found". - Updates the method docstring to warn users about this dynamic replacement. ## Related issue number Fixes ray-project#63669 --------- Signed-off-by: Ayush KAshyap <kashyap11ayush02@gmail.com>

## Description `linux://rllib:examples/ray_tune/appo_hyperparameter_tune` was failing with ``` ==================== Test output for //rllib:examples/ray_tune/appo_hyperparameter_tune: Traceback (most recent call last): File "rllib/examples/ray_tune/appo_hyperparameter_tune.py", line 155, in <module> tuner = tune.Tuner( ^^^^^^^^^^^ File "/rayci/python/ray/tune/tuner.py", line 135, in __init__ self._local_tuner = TunerInternal(**kwargs) ^^^^^^^^^^^^^^^^^^^^^^^ File "/rayci/python/ray/tune/impl/tuner_internal.py", line 161, in __init__ self.param_space = param_space ^^^^^^^^^^^^^^^^ File "/rayci/python/ray/tune/impl/tuner_internal.py", line 502, in param_space param_space = param_space.to_dict() ^^^^^^^^^^^^^^^^^^^^^ File "/rayci/python/ray/rllib/algorithms/algorithm_config.py", line 753, in to_dict config["train_batch_size"] = self.total_train_batch_size ^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/rayci/python/ray/rllib/algorithms/algorithm_config.py", line 4321, in total_train_batch_size return self.train_batch_size_per_learner * (self.num_learners or 1) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~ TypeError: unsupported operand type(s) for *: 'Integer' and 'int' ================================================================================ ``` #63695 introduced a property for `total_train_batch_size` however this causes an issue for RLlib + Ray Tune. This PR largely reverts the #63695 and adds a check that a Tune parameter is usable Signed-off-by: Mark Towers <mark@anyscale.com> Co-authored-by: Mark Towers <mark@anyscale.com>

## Description `linux://rllib:examples/ray_tune/appo_hyperparameter_tune` was failing with ``` ==================== Test output for //rllib:examples/ray_tune/appo_hyperparameter_tune: Traceback (most recent call last): File "rllib/examples/ray_tune/appo_hyperparameter_tune.py", line 155, in <module> tuner = tune.Tuner( ^^^^^^^^^^^ File "/rayci/python/ray/tune/tuner.py", line 135, in __init__ self._local_tuner = TunerInternal(**kwargs) ^^^^^^^^^^^^^^^^^^^^^^^ File "/rayci/python/ray/tune/impl/tuner_internal.py", line 161, in __init__ self.param_space = param_space ^^^^^^^^^^^^^^^^ File "/rayci/python/ray/tune/impl/tuner_internal.py", line 502, in param_space param_space = param_space.to_dict() ^^^^^^^^^^^^^^^^^^^^^ File "/rayci/python/ray/rllib/algorithms/algorithm_config.py", line 753, in to_dict config["train_batch_size"] = self.total_train_batch_size ^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/rayci/python/ray/rllib/algorithms/algorithm_config.py", line 4321, in total_train_batch_size return self.train_batch_size_per_learner * (self.num_learners or 1) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~ TypeError: unsupported operand type(s) for *: 'Integer' and 'int' ================================================================================ ``` #63695 introduced a property for `total_train_batch_size` however this causes an issue for RLlib + Ray Tune. This PR largely reverts the #63695 and adds a check that a Tune parameter is usable Signed-off-by: Mark Towers <mark@anyscale.com> Co-authored-by: Mark Towers <mark@anyscale.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>

AyushKashyapII requested a review from a team as a code owner May 28, 2026 11:51

[RLlib] Fix AlgorithmConfig.to_dict() for New API Stack

33a2a7e

Signed-off-by: Ayush KAshyap <kashyap11ayush02@gmail.com>

AyushKashyapII force-pushed the rllib/fix-to-dict-batch-size-#63669 branch from f7efa84 to 33a2a7e Compare May 28, 2026 11:52

gemini-code-assist Bot reviewed May 28, 2026

View reviewed changes

cursor Bot reviewed May 28, 2026

View reviewed changes

Comment thread rllib/algorithms/algorithm_config.py Outdated

Remove read-only total_train_batch_size from to_dict to prevent updat…

3ef559a

…e_from_dict crash Signed-off-by: Ayush KAshyap <kashyap11ayush02@gmail.com>

AyushKashyapII force-pushed the rllib/fix-to-dict-batch-size-#63669 branch from 8cc7d60 to 3ef559a Compare May 28, 2026 12:20

cursor Bot reviewed May 28, 2026

View reviewed changes

Comment thread rllib/algorithms/algorithm_config.py Outdated

ray-gardener Bot added rllib RLlib related issues docs An issue or change related to documentation community-contribution Contributed by the community labels May 28, 2026

cursor Bot reviewed May 28, 2026

View reviewed changes

Comment thread rllib/algorithms/algorithm_config.py Outdated

pseudo-rnd-thoughts requested changes Jun 1, 2026

View reviewed changes

[RLlib] Add roundtripping test for to_dict on New API Stack

463d244

Signed-off-by: Ayush KAshyap <kashyap11ayush02@gmail.com>

AyushKashyapII force-pushed the rllib/fix-to-dict-batch-size-#63669 branch from 87e1a78 to 463d244 Compare June 1, 2026 11:47

AyushKashyapII force-pushed the rllib/fix-to-dict-batch-size-#63669 branch from 6c9dd26 to bb3c545 Compare June 2, 2026 11:59

Fix trailing whitespace and missing dict key in to_dict()

4694437

Signed-off-by: Ayush KAshyap <kashyap11ayush02@gmail.com>

AyushKashyapII force-pushed the rllib/fix-to-dict-batch-size-#63669 branch from bb3c545 to 4694437 Compare June 2, 2026 12:02

pseudo-rnd-thoughts approved these changes Jun 2, 2026

View reviewed changes

Fix TypeError when train_batch_size is None and run formatter

ff782ea

Signed-off-by: Ayush KAshyap <kashyap11ayush02@gmail.com>

pseudo-rnd-thoughts reviewed Jun 4, 2026

View reviewed changes

AyushKashyapII force-pushed the rllib/fix-to-dict-batch-size-#63669 branch from fe37009 to 41241ea Compare June 4, 2026 12:28

Adding ValueError in to_dict()

8cfc328

Signed-off-by: Ayush KAshyap <kashyap11ayush02@gmail.com>

AyushKashyapII force-pushed the rllib/fix-to-dict-batch-size-#63669 branch from 41241ea to 8cfc328 Compare June 4, 2026 12:29

Merge branch 'master' into rllib/fix-to-dict-batch-size-#63669

df2dff9

pseudo-rnd-thoughts reviewed Jun 12, 2026

View reviewed changes

pseudo-rnd-thoughts removed the docs An issue or change related to documentation label Jun 12, 2026

AyushKashyapII added 2 commits June 12, 2026 15:17

Merge branch 'master' into rllib/fix-to-dict-batch-size-#63669

cf696fd

Merge branch 'master' into rllib/fix-to-dict-batch-size-#63669

a292823

AyushKashyapII requested a review from pseudo-rnd-thoughts June 15, 2026 15:17

Merge branch 'master' into rllib/fix-to-dict-batch-size-#63669

8d3c4fc

pseudo-rnd-thoughts added the go add ONLY when ready to merge, run all tests label Jun 23, 2026

pseudo-rnd-thoughts approved these changes Jun 23, 2026

View reviewed changes

ArturNiederfahrenhorst enabled auto-merge (squash) June 23, 2026 12:47

ArturNiederfahrenhorst merged commit 7c507fc into ray-project:master Jun 23, 2026
9 checks passed

pseudo-rnd-thoughts mentioned this pull request Jul 2, 2026

[Rllib] Fix RLlib with Tune (revert #63695) #64501

Merged

Uh oh!

Conversation

AyushKashyapII commented May 28, 2026

Why are these changes needed?

Related issue number

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AyushKashyapII commented May 28, 2026

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pseudo-rnd-thoughts left a comment

Choose a reason for hiding this comment

Uh oh!

AyushKashyapII commented Jun 1, 2026

Uh oh!

pseudo-rnd-thoughts left a comment

Choose a reason for hiding this comment

Uh oh!

pseudo-rnd-thoughts commented Jun 2, 2026

Uh oh!

AyushKashyapII commented Jun 2, 2026

Uh oh!

pseudo-rnd-thoughts left a comment

Choose a reason for hiding this comment

Uh oh!

pseudo-rnd-thoughts Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

AyushKashyapII Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

pseudo-rnd-thoughts Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

AyushKashyapII commented Jun 9, 2026

Uh oh!

pseudo-rnd-thoughts left a comment

Choose a reason for hiding this comment

Uh oh!

pseudo-rnd-thoughts Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

AyushKashyapII Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

AyushKashyapII commented Jun 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants