pku-alignment/safe-policy-optimization

NeurIPS 2023: Safe Policy Optimization: A benchmark repository for safe reinforcement learning algorithms

safe-policy-optimization's Issues

Question about the implementation of the IPO

I am confused about the implementation of the IPO. The original paper uses the logarithm barrier function to approximate the indicator function. But 'log' does not appear in this code

question about env reset

obs, _ = env.reset()
obs = torch.as_tensor(obs, dtype=torch.float32, device=device)
ep_ret, ep_cost, ep_len = (
    np.zeros(args.num_envs),
    np.zeros(args.num_envs),
    np.zeros(args.num_envs),
)
# training loop
for epoch in range(epochs):
    rollout_start_time = time.time()
    # collect samples until we have enough to update
    for steps in range(local_steps_per_epoch):  

Why did your code only perform env.reset() at the beginning, rather than starting at each epoch？

Something about IPO

Hello, thanks for your excellent work. For the IPO algorithm, I wonder that if it is suit for the environment whose action space is discrete?
Because of that the interior point algorithm is not suit for the optimization problem whose decisision is discrete.

Safexp-PointGoal1-v0 vs SafetyPointGoal1-v0

Dear,

what is the difference between Safexp-PointGoal1-v0 vs SafetyPointGoal1-v0?

I find that, result from SafetyPointGoal1-v0 is generally smaller than Safexp-PointGoal1-v0

Doubt about the updating method of Lagrange Multipliers

from safepo.common.lagrange import Lagrange

nu = 1.0
nu_lr = 0.1
ep_cost = 35
lagrange = Lagrange(
cost_limit=25.0,
lagrangian_multiplier_init=1.0,
lagrangian_multiplier_lr=0.1,
)

print("Before update:")
print(f"Learning Rate: {lagrange.lambda_optimizer.param_groups[0]['lr']}")

lagrange.update_lagrange_multiplier(ep_cost)

learn_lag = lagrange.lagrangian_multiplier

nu += nu_lr * (ep_cost - 25.0)

print(f"Lagrange multiplier: {learn_lag}")
print(f"Nu: {nu}")

There are two methods: one is to treat it as a learnable parameter and use the Adam optimizer, and the other is to directly assign values. Due to the adaptive learning rate of the Adam optimizer, the multipliers obtained by the two are not consistent, and the multiplier changes rapidly in the case of direct assignment under the same learning rate.

I understand that the original code of PPO Lagrangian uses the former method, while the original code of FOCOPS and CUP seems to use the latter method. Should it be distinguished or can the former be used uniformly?

Process conflict casused abnormal termination

1. Follow the installation instructions to install
2. An exception occurred while executing this command:
   python safepo/train.py --env_id Safexp-PointGoal1-v0 --algo ppo_lagrangian --cores 4
3. Exception prompt:

Traceback (most recent call last):
File "safepo/train.py", line 61, in
model = Runner(
File "/home/jiayiguan/opt/paper_carla/Safe-Policy-Optimization/safepo/common/runner.py", line 36, in init
self.default_kwargs = get_defaults_kwargs_yaml(algo=algo,env_id=env_id)
File "/home/jiayiguan/opt/paper_carla/Safe-Policy-Optimization/safepo/common/utils.py", line 15, in get_defaults_kwargs_yaml
return kwargs[kwargs_name]
KeyError: 'defaults'

Primary job terminated normally, but 1 process returned
a non-zero exit code.. Per user-direction, the job has been aborted.

mpirun detected that one or more processes exited with non-zero status, thus causing
the job to be terminated. The first process to do so was:

Process name: [[52236,1],0]
Exit code: 1

Question about the torch.size of loss_pi in focops implement

I found in focops.py :
ratio = torch.exp(log_prob - log_prob_b)
temp_kl = torch.distributions.kl_divergence(
distribution, old_distribution_b
).sum(-1, keepdim=True)
loss_pi = (temp_kl - (1 / FOCOPS_LAM) * ratio * adv_b) * (
temp_kl.detach() <= dict_args['target_kl']
).type(torch.float32)
loss_pi = loss_pi.mean()
Assuming minibatch size=64, temp_kl.shape=(64,1) due to keepdim=True used in calculating temp_kl, but other variables in loss_pi = (64,) which makes loss_pi.shape=(64,64) instead of (64,1) or (64,). So, Why not keep the same dimensions？

Question about logger value

                if done or time_out:
                    rew_deque.append(ep_ret[idx])
                    cost_deque.append(ep_cost[idx])
                    len_deque.append(ep_len[idx])
                    logger.store(
                        **{
                            "Metrics/EpRet": np.mean(rew_deque),
                            "Metrics/EpCost": np.mean(cost_deque),
                            "Metrics/EpLen": np.mean(len_deque),
                        }
                    )
                    ep_ret[idx] = 0.0
                    ep_cost[idx] = 0.0
                    ep_len[idx] = 0.0

I'm confused about this np.mean(cost_deque), this would make the EpCost for different epochs correlate, making ep_costs = logger.get_stats("Metrics/EpCost") different from the Jc definition in the safe RL paper. The purpose of this is to utilize the data from many previous episodes to average out the newly added data, which helps improve training stability, and will make the drawn training curve look smoother?

why?

Traceback (most recent call last):
File "train.py", line 54, in
if mpi_tools.mpi_fork(args.cores,use_number_of_threads=use_number_of_threads):
File "/content/drive/MyDrive/save/Safe-Policy-Optimization/safepo/common/mpi_tools.py", line 97, in mpi_fork
subprocess.check_call(args, env=env)
File "/usr/lib/python3.7/subprocess.py", line 363, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['mpirun', '-np', '2', '--use-hwthread-cpus', '/usr/bin/python3', 'train.py', '--env-id', 'Safexp-PointGoal1-v0', '--algo', 'ppo-lag', '--cores', '2', '--seed', '0']' returned non-zero exit status 1.
[5e506354ec7b:06923] *** Process received signal ***
why i have this question?
please help me,thank you

Question about rescale in cpo

    b_flat = get_flat_gradients_from(self.ac.pi.net)

    ep_costs = self.logger.get_stats('EpCosts')[0]
    c = ep_costs - self.cost_limit
    c /= (self.logger.get_stats('EpLen')[0] + eps)  # rescale
    self.logger.log(f'c = {c}')
    self.logger.log(f'b^T b = {b_flat.dot(b_flat).item()}')

Why we needs to rescale here?