Skip to content

[BUG] GAEConfig.average_gae default does not match GAE.__init__ #4463

Description

@YeonwooSung

Describe the bug

GAEConfig.average_gae defaults to True, but GAE.__init__ defaults average_gae to False (class docstring: "Default is False").

GAEConfig documents that every GAE.__init__ kwarg is exposed as a field. Hydra instantiate(GAEConfig) calls _make_gaeGAE.__init__, so leaving the field unset standardizes advantages even though constructing GAE(...) does not.

This is a default-value mismatch on an existing field, not a missing Config field. It is independent of the _CONFIG_PARITY_KNOWN_GAPS tracker in #4228 (GAEConfig is not on that allowlist; default equality is not enforced there).

GAE.for_recurrent defaults average_gae to True on purpose. That constructor is not used by instantiate(GAEConfig) and should stay as-is.

To Reproduce

from hydra.utils import instantiate
from torchrl.objectives.value import GAE
from torchrl.trainers.algorithms.configs.objectives import GAEConfig

assert GAE(gamma=0.99, lmbda=0.95, value_network=None).average_gae is False
cfg = GAEConfig(gamma=0.99, lmbda=0.95)
assert cfg.average_gae is True  # Config default
assert instantiate(cfg).average_gae is True  # Hydra users get GAE(average_gae=True)

Expected behavior

GAEConfig.average_gae should default to False, matching GAE.__init__. instantiate(GAEConfig(gamma=..., lmbda=...)) should produce an estimator with average_gae is False.

Additional context

    average_gae: bool = True
        average_gae: bool = False,
        average_gae (bool): if ``True``, the resulting GAE values will be standardized.
            Default is ``False``.

test_gae_config_instantiates_nested_group_key instantiates GAEConfig but does not assert this default.

Reason and Possible fixes

Set GAEConfig.average_gae = False so it matches GAE.__init__. Keep GAE.for_recurrent(..., average_gae=True).

Checklist

  • I have checked that there is no similar issue in the repo (required)
  • I have read the documentation (required)
  • I have provided a minimal working example to reproduce the bug (required)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions