PKU-Alignment / omnisafe

JMLR: OmniSafe is an infrastructural framework for accelerating SafeRL research.
https://www.omnisafe.ai
Apache License 2.0
948 stars 132 forks source link

[BUG] Can't we specify `standardized_rew_adv` and `standardized_cost_adv` at the same time? #173

Closed r-y1 closed 1 year ago

r-y1 commented 1 year ago

Required prerequisites

What version of OmniSafe are you using?

0.1.0

System information

no need

Problem description

Can't we specify standardized_rew_adv and standardized_cost_adv at the same time? I guess it is a bug.

b6c6d67014cc5a8e0d9ff485a101ca5

https://github.com/PKU-MARL/omnisafe/blob/main/omnisafe/utils/config.py#L216

Reproducible example code

The Python snippets:

Command lines:

Extra dependencies:

Steps to reproduce:

1. 2. 3.

Traceback

No response

Expected behavior

No response

Additional context

No response

Gaiejj commented 1 year ago

We apologize for the problem, it was a simple typo. standardized_cost_adv and standardized_rew_adv are usually a generic trick that defaults to True in OmniSafe, but should not be forced to be specified as True.The correct way to write it should be

assert (
     isinstance(configs.standardized_rew_adv, bool) 
     and isinstance(configs.standardized_cost_adv, bool)
 ), 'standardized_<>_adv must be bool'

We'll fix this as soon as possible, thanks for your feedback!