Merge pull request #574 from BDonnot/bd_dev

Fix MaskedEnvironment

CHANGELOG.rst

@@ -39,6 +39,7 @@ Change Log
 - [FIXED] `PandapowerBackend`, when no slack was present
 - [FIXED] the "BaseBackendTest" class did not correctly detect divergence in most cases (which lead 
   to weird bugs in failing tests)
+- [FIXED] an issue with imageio having deprecated the `fps` kwargs (see https://github.com/rte-france/Grid2Op/issues/569)
 - [ADDED] A type of environment that does not perform the "emulation of the protections"
   for some part of the grid (`MaskedEnvironment`) see https://github.com/rte-france/Grid2Op/issues/571
 - [IMPROVED] the CI speed: by not testing every possible numpy version but only most ancient and most recent

docs/action.rst

@@ -85,7 +85,7 @@ you want to perform on the grid. For more information you can consult the help o
 
 To avoid extremely verbose things, as of grid2op 1.5.0, we introduced some convenience functions to allow
 easier action construction. You can now do `act.load_set_bus = ...` instead of the previously way
-more verbose `act.update({"set_bus": {"loads_id": ...}}`
+more verbose `act.update({"set_bus": {"loads_id": ...}})`
 
 .. _action-module-examples:
 

docs/environment.rst

@@ -101,7 +101,7 @@ be equivalent to starting into the "middle" of a video game. If that is the case
 Finally, you might have noticed that each call to "env.reset" might take a while. This can dramatically
 increase the training time, especially at the beginning. This is due to the fact that each time
 `env.reset` is called, the whole chronics is read from the hard drive. If you want to lower this
-impact then you might consult the `Optimize the data pipeline`_ section.
+impact then you might consult the :ref:`environment-module-data-pipeline` page of the doc.
 
 .. _environment-module-chronics-info:
 

grid2op/Environment/baseEnv.py

@@ -84,6 +84,65 @@ class BaseEnv(GridObjects, RandomObject, ABC):
 
     The documentation is showed here to document the common attributes of an "BaseEnvironment".
 
+    .. _danger-env-ownership:
+    
+    Notes 
+    ------------------------
+    
+    Note en environment data ownership
+    
+    .. danger::
+    
+        
+        A non pythonic decision has been implemented in grid2op for various reasons: an environment
+        owns everything created from it.
+        
+        This means that if you (or the python interpreter) deletes the environment, you might not
+        use some data generate with this environment.
+        
+        More precisely, you cannot do something like:
+        
+        .. code-block:: python
+
+            import grid2op
+            env = grid2op.make("l2rpn_case14_sandbox")
+            
+            saved_obs = []
+            
+            obs = env.reset()
+            saved_obs.append(obs)
+            obs2, reward, done, info = env.step(env.action_space())
+            saved_obs.append(obs2)
+            
+            saved_obs[0].simulate(env.action_space())  # works
+            del env
+            saved_obs[0].simulate(env.action_space())  # DOES NOT WORK
+            
+        It will raise an error like `Grid2OpException EnvError "This environment is closed. You cannot use it anymore."`
+        
+        This will also happen if you do things inside functions, for example like this:
+        
+        .. code-block:: python
+
+            import grid2op
+            
+            def foo(manager):
+                env = grid2op.make("l2rpn_case14_sandbox")
+                obs = env.reset()
+                manager.append(obs)
+                obs2, reward, done, info = env.step(env.action_space())
+                manager.append(obs2)
+                manager[0].simulate(env.action_space())  # works
+                return manager
+                
+            manager = []
+            manager = foo(manager)
+            manager[0].simulate(env.action_space())  # DOES NOT WORK
+        
+        The same error is raised because the environment `env` is automatically deleted by python when the function `foo` ends
+        (well it might work on some cases, if the function is called before the variable `env` is actually deleted but you 
+        should not rely on this behaviour.)
+            
     Attributes
     ----------
 

grid2op/Environment/environment.py

@@ -37,6 +37,14 @@ class Environment(BaseEnv):
     """
     This class is the grid2op implementation of the "Environment" entity in the RL framework.
 
+    .. danger::
+    
+        Long story short, once a environment is deleted, you cannot use anything it "holds" including,
+        but not limited to the capacity to perform `obs.simulate(...)` even if the `obs` is still
+        referenced.
+        
+        See :ref:`danger-env-ownership` (first danger block). 
+        
     Attributes
     ----------
 

grid2op/Environment/maskedEnvironment.py

@@ -27,7 +27,14 @@ class MaskedEnvironment(Environment):  # TODO heritage ou alors on met un truc d
     .. warning::
         At time of writing, the behaviour of "obs.simulate" is not modified
     """  
-    CAN_SKIP_TS = False  # some steps can be more than one time steps
+    # some kind of infinity value
+    # NB we multiply np.finfo(dt_float).max by a small number (1e-7) to avoid overflow
+    # indeed, _hard_overflow_threshold is multiply by the flow on the lines
+    INF_VAL_THM_LIM = 1e-7 * np.finfo(dt_float).max  
+
+    # some kind of infinity value
+    INF_VAL_TS_OVERFLOW_ALLOW = np.iinfo(dt_int).max - 1  
+
     def __init__(self,
                  grid2op_env: Union[Environment, dict],
                  lines_of_interest):
@@ -38,7 +45,7 @@ def __init__(self,
         elif isinstance(grid2op_env, dict):
             super().__init__(**grid2op_env)
         else:
-            raise EnvError(f"For TimedOutEnvironment you need to provide "
+            raise EnvError(f"For MaskedEnvironment you need to provide "
                            f"either an Environment or a dict "
                            f"for grid2op_env. You provided: {type(grid2op_env)}")
 
@@ -62,10 +69,8 @@ def _make_lines_of_interest(self, lines_of_interest):
 
     def _reset_vectors_and_timings(self):
         super()._reset_vectors_and_timings()
-        self._hard_overflow_threshold[~self._lines_of_interest] = 1e-7 * np.finfo(dt_float).max   # some kind of infinity value
-        # NB we multiply np.finfo(dt_float).max by a small number to avoid overflow
-        # indeed, _hard_overflow_threshold is multiply by the flow on the lines
-        self._nb_timestep_overflow_allowed[~self._lines_of_interest] = np.iinfo(dt_int).max - 1  # some kind of infinity value
+        self._hard_overflow_threshold[~self._lines_of_interest] = type(self).INF_VAL_THM_LIM
+        self._nb_timestep_overflow_allowed[~self._lines_of_interest] = type(self).INF_VAL_TS_OVERFLOW_ALLOW
 
     def get_kwargs(self, with_backend=True, with_chronics_handler=True):
         res = {}
@@ -79,6 +84,10 @@ def get_params_for_runner(self):
         res["other_env_kwargs"] = {"lines_of_interest": copy.deepcopy(self._lines_of_interest)}
         return res
 
+    def _custom_deepcopy_for_copy(self, new_obj):
+        super()._custom_deepcopy_for_copy(new_obj)
+        new_obj._lines_of_interest = copy.deepcopy(self._lines_of_interest)
+
     @classmethod
     def init_obj_from_kwargs(cls,
                              other_env_kwargs,

grid2op/Episode/EpisodeReplay.py

@@ -102,15 +102,15 @@ def replay_episode(
 
         load_info: ``str``
             Defaults to "p". What kind of values to show on loads.
-            Can be oneof `["p", "v", None]`
+            Can be one of `["p", "v", None]`
 
         gen_info: ``str``
             Defaults to "p". What kind of values to show on generators.
-            Can be oneof `["p", "v", None]`
+            Can be one of `["p", "v", None]`
 
         line_info: ``str``
             Defaults to "rho". What kind of values to show on lines.
-            Can be oneof `["rho", "a", "p", "v", None]`
+            Can be one of `["rho", "a", "p", "v", None]`
 
         resolution: ``tuple``
             Defaults to (1280, 720). The resolution to use for the gif.
@@ -187,7 +187,12 @@ def replay_episode(
         # Export all frames as gif if enabled
         if gif_name is not None and len(frames) > 0:
             try:
-                imageio.mimwrite(gif_path, frames, fps=fps)
+                try:
+                    # with imageio > 2.5 you need to compute the duration
+                    imageio.mimwrite(gif_path, frames, duration=1000./fps)
+                except TypeError:
+                    # imageio <= 2.5 can be given fps directly
+                    imageio.mimwrite(gif_path, frames, fps=fps)
                 # Try to compress
                 try:
                     from pygifsicle import optimize

grid2op/Observation/baseObservation.py

@@ -4207,7 +4207,18 @@ def get_forecast_env(self) -> "grid2op.Environment.Environment":
                 f_obs_3, *_ = forecast_env.step(act_3)
                 sim_obs_3, *_ = sim_obs_2.simulate(act_3)
                 # f_obs_3 should be sim_obs_3
-                
+        
+        .. danger::
+    
+            Long story short, once a environment (and a forecast_env is one) 
+            is deleted, you cannot use anything it "holds" including,
+            but not limited to the capacity to perform `obs.simulate(...)` even if the `obs` is still
+            referenced.
+
+            See :ref:`danger-env-ownership` (first danger block). 
+            
+            This caused issue https://github.com/rte-france/Grid2Op/issues/568 for example.
+            
         Returns
         -------
         grid2op.Environment.Environment
@@ -4339,8 +4350,26 @@ def get_env_from_external_forecasts(self,
             you have 100 rows then you have 100 steps. 
         
         .. warning::
-            We remind that, if you provide some forecasts, it is expected that 
+            We remind that, if you provide some forecasts, it is expected that they allow some powerflow to converge.
+            The balance between total generation on one side and total demand and losses on the other should also
+            make "as close as possible" to reduce some modeling artifact (by the backend, grid2op does not check
+            anything here).
+            
+            Finally, make sure that your input data meet the constraints on the generators (pmin, pmax and ramps)
+            otherwise you might end up with incorrect behaviour. Grid2op supposes that data fed to it
+            is consistent with its model. If not it's "undefined behaviour".
+        
+        .. danger::
+    
+            Long story short, once a environment (and a forecast_env is one) 
+            is deleted, you cannot use anything it "holds" including,
+            but not limited to the capacity to perform `obs.simulate(...)` even if the `obs` is still
+            referenced.
+
+            See :ref:`danger-env-ownership` (first danger block). 
             
+            This caused issue https://github.com/rte-france/Grid2Op/issues/568 for example.
+             
         Examples
         --------
         A typical use might look like

grid2op/Runner/runner.py

@@ -1137,6 +1137,7 @@ def run(
                 returned list are not necessarily sorted by this value)
               - "cum_reward" the cumulative reward obtained by the :attr:`Runner.Agent` on this episode i
               - "nb_time_step": the number of time steps played in this episode.
+              - "total_step": the total number of time steps possible in this episode.
               - "episode_data" : [Optional] The :class:`EpisodeData` corresponding to this episode run only
                 if `add_detailed_output=True`
               - "add_nb_highres_sim": [Optional] The estimated number of calls to high resolution simulator made

grid2op/tests/BaseBackendTest.py

@@ -2741,7 +2741,7 @@ def test_issue_134(self):
             }
         )
         obs, reward, done, info = env.step(action)
-        assert not done
+        assert not done, f"Episode should not have ended here, error : {info['exception']}"
         assert obs.line_status[LINE_ID] == False
         assert obs.topo_vect[obs.line_or_pos_topo_vect[LINE_ID]] == -1
         assert obs.topo_vect[obs.line_ex_pos_topo_vect[LINE_ID]] == -1