Skip to content

Warn if using offline data buffer without Agent wrapper #1771

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.

Sign up for GitHub

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

Open
QuantuMope wants to merge 3 commits into
base: pytorch
Choose a base branch
from
Open

Warn if using offline data buffer without Agent wrapper #1771

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
74240c4
[Bug Fix] Enable hybrid SAC training
QuantuMope May 12, 2025
2c7e7cf
address comments
QuantuMope May 12, 2025
9db5652
address comments
QuantuMope May 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions alf/algorithms/sac_algorithm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@
from alf.algorithms.config import TrainerConfig
from alf.algorithms.off_policy_algorithm import OffPolicyAlgorithm
from alf.algorithms.one_step_loss import OneStepTDLoss
from alf.algorithms.rl_algorithm import RLAlgorithm
from alf.data_structures import TimeStep, Experience, LossInfo, namedtuple
from alf.data_structures import TimeStep, LossInfo, namedtuple, \
BasicRolloutInfo
from alf.data_structures import AlgStep, StepType
from alf.nest import nest
import alf.nest.utils as nest_utils
from alf.networks import ActorDistributionNetwork, CriticNetwork
from alf.networks import QNetwork, QRNNNetwork
from alf.networks import QNetwork
from alf.tensor_specs import TensorSpec, BoundedTensorSpec
from alf.utils import losses, common, dist_utils, math_ops
from alf.utils.normalizers import ScalarAdaptiveNormalizer
Expand Down Expand Up @@ -847,6 +847,10 @@ def _select_q_value(self, action, q_values):
def _critic_train_step(self, observation, target_observation,
state: SacCriticState, rollout_info: SacInfo,
action, action_distribution):

if isinstance(rollout_info, BasicRolloutInfo):
rollout_info = rollout_info.rl
Copy link
Collaborator

@hnyu hnyu May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be put outside of this function. The general principle is, the algorithm should always receive what it's supposed to receive. In this case, this means that the rollout_info passed in should already be SacInfo.

Copy link
Contributor

@emailweixu emailweixu May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be put outside of this function. The general principle is, the algorithm should always receive what it's supposed to receive. In this case, this means that the rollout_info passed in should already be SacInfo.

+1

Copy link
Contributor Author

@QuantuMope QuantuMope May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Fixed.


critics, critics_state = self._compute_critics(
self._critic_networks,
observation,
Expand Down