Skip to content
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.

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

obs.simulate and env.step check cooldown differently #374

Closed
richardwth opened this issue Nov 8, 2022 · 4 comments · Fixed by #383
Closed

obs.simulate and env.step check cooldown differently #374

richardwth opened this issue Nov 8, 2022 · 4 comments · Fixed by #383
Labels
bug Something isn't working

Comments

@richardwth
Copy link

Environment

  • Grid2op version: 1.7.2
  • System: ubuntu

Bug description

There are two types of cooldown in the game: time_before_cooldown_sub and time_before_cooldown_line. When cooldown of a sub or a line is 1, obs.simulate thinks an action on this sub or line is OK, but env.step thinks it is illegal.

How to reproduce

import grid2op
from lightsim2grid import LightSimBackend as bk_cls
from grid2op.Parameters import Parameters

# First, create env with cooldown
param = Parameters()
param.NB_TIMESTEP_COOLDOWN_SUB = 3
param.NB_TIMESTEP_COOLDOWN_LINE = 3
env = grid2op.make('l2rpn_wcci_2022', param=param, backend=bk_cls())
action_space = env.action_space
do_nothing = action_space({})

# second, get some action that has no actual effect on the grid
set_sub_10 = action_space.get_all_unitary_topologies_set(action_space, sub_id=10)
set_sub10_to_bus1 = set_sub_10[0]
# can also get some line action
# line_actions = action_space.get_all_unitary_line_set_simple(action_space)
# connect_last_line = line_actions[-1]

# last, do the action multiple times
_ = env.reset()

obs, _, done, info = env.step(set_sub10_to_bus1)
print('Step {}:'.format(obs.current_step), obs.time_before_cooldown_sub[10], info['exception'])

obs, _, done, info = env.step(do_nothing)
print('Step {}:'.format(obs.current_step), obs.time_before_cooldown_sub[10], info['exception'])
obs_, _, done, info = obs.simulate(set_sub10_to_bus1)
print('Step {} simu:'.format(obs.current_step), info['exception'])

obs, _, done, info = env.step(do_nothing)
print('Step {}:'.format(obs.current_step), obs.time_before_cooldown_sub[10], info['exception'])
obs_, _, done, info = obs.simulate(set_sub10_to_bus1)
print('Step {} simu:'.format(obs.current_step), info['exception'])

obs, _, done, info = env.step(set_sub10_to_bus1)
print('Step {}:'.format(obs.current_step), obs.time_before_cooldown_sub[10], info['exception'])

obs, _, done, info = env.step(set_sub10_to_bus1)
print('Step {}:'.format(obs.current_step), obs.time_before_cooldown_sub[10], info['exception'])

Current output

Step 1: 3 []
Step 2: 2 []
Step 2 simu: [Grid2OpException IllegalAction IllegalAction('Substation with ids [10] have been modified illegally (cooldown)')]
Step 3: 1 []
Step 3 simu: []
Step 4: 0 [Grid2OpException IllegalAction IllegalAction('Substation with ids [10] have been modified illegally (cooldown)')]
Step 5: 3 []

Expected output

I expect obs.simulate and env.step to be consistent on cooldown check of sub or line. For example, following the logic of obs.simulate, the output should be:

Step 1: 3 []
Step 2: 2 []
Step 2 simu: [Grid2OpException IllegalAction IllegalAction('Substation with ids [10] have been modified illegally (cooldown)')]
Step 3: 1 []
Step 3 simu: []
Step 4: 3 []
Step 5: 2 [Grid2OpException IllegalAction IllegalAction('Substation with ids [10] have been modified illegally (cooldown)')]
@richardwth richardwth added the bug Something isn't working label Nov 8, 2022
@BDonnot
Copy link
Collaborator

BDonnot commented Nov 8, 2022

Hello,

Indeed that's not a correct behavior. I'll have a look and implement a fix for the new release (hopefully sooner rather than later)

Thanks for the issue 😊

@richardwth
Copy link
Author

May I ask which one is the correct behavior by design, obs.simulate and env.step?

@BDonnot
Copy link
Collaborator

BDonnot commented Nov 8, 2022

Cooldowns are carefully checked and tested for the env.step method.

Unless there is an obvious bug in env.step (which can happen of course, this software, like every other is not perfect) it should be used as the reference.

So env.step is correct in this case.

BDonnot added a commit to BDonnot/Grid2Op that referenced this issue Dec 5, 2022
@BDonnot BDonnot linked a pull request Dec 12, 2022 that will close this issue
@BDonnot
Copy link
Collaborator

BDonnot commented Dec 12, 2022

Bug should be fixed, and i clarify the doc for the correct behaviour, see https://grid2op.readthedocs.io/en/latest/rules.html#behaviour and following subsection for more information :-)

@BDonnot BDonnot closed this as completed Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants