Merge pull request #448 from BDonnot/master

Add some security when using MultiFolderWithCache

CHANGELOG.rst

@@ -126,6 +126,8 @@ Change Log
 - [IMPROVED] add the possibility to forward kwargs to chronix2grid function when calling `env.generate_data`
 - [IMPROVED] when calling `env.generate_data` an extra file (json) will be read to set default values 
   passed to `chronix2grid.add_data`
+- [IMPROVED] it is no more reasonably possible to misuse the `MultifolderWithCache` (for example by
+  forgetting to `reset()` the cache): an error will be raised in case the proper function has not been called.
 
 [1.8.1] - 2023-01-11
 ---------------------

docs/data_pipeline.rst

@@ -113,7 +113,16 @@ Another way is to use a dedicated class that stores the data in memory. This is
 to avoid long and inefficient I/O that are replaced by reading the the complete dataset once and store it
 into memory.
 
-.. warning::
+.. seealso::
+    The documentation of :class:`grid2op.Chronics.Chronics.MultifolderWithCache` for a more
+    detailed documentation.
+
+.. versionchanged:: 1.8.2
+    Any call to "env.reset()" or "env.step()" without a previous call to `env.chronics_handler.real_data.reset()`
+    will raise an error preventing any use of the environment.
+    (It is no longer assumed people read, at least partially the documentation.)
+
+.. danger::
     When you create an environment with this chronics class (*eg* by doing 
     `env = make(...,chronics_class=MultifolderWithCache)`), the "cache" is not
     pre loaded, only the first scenario is loaded in memory (to save loading time).

grid2op/Chronics/multifolderWithCache.py

@@ -11,6 +11,7 @@
 from grid2op.dtypes import dt_int
 from grid2op.Chronics.multiFolder import Multifolder
 from grid2op.Chronics.gridStateFromFile import GridStateFromFile
+from grid2op.Exceptions import ChronicsError
 
 
 class MultifolderWithCache(Multifolder):
@@ -30,7 +31,7 @@ class MultifolderWithCache(Multifolder):
     performs less than a few dozen of steps leading to more time spent reading 8600 rows than computing the
     few dozen of steps.
 
-    .. warning::
+    .. danger::
         When you create an environment with this chronics class (*eg* by doing 
         `env = make(...,chronics_class=MultifolderWithCache)`), the "cache" is not
         pre loaded, only the first scenario is loaded in memory (to save loading time).
@@ -86,7 +87,13 @@ class MultifolderWithCache(Multifolder):
 
     """
     MULTI_CHRONICS = True
-
+    ERROR_MSG_NOT_LOADED = ("We detected a misusage of the `MultifolderWithCache` class: the cache "
+                            "has not been loaded in memory which will most likely cause issues "
+                            "with your environment. Do not forget to call "
+                            "`env.chronics_handler.set_filter(...)` to tell which time series "
+                            "you want to keep and then `env.chronics_handler.reset()` "
+                            "to load them. \nFor more information consult the documentation:\n"
+                            "https://grid2op.readthedocs.io/en/latest/chronics.html#grid2op.Chronics.MultifolderWithCache")
     def __init__(
         self,
         path,
@@ -99,6 +106,26 @@ def __init__(
         filter_func=None,
         **kwargs,
     ):
+
+        # below: counter to prevent use without explicit call to `env.chronics.handler.reset()`
+        if "_DONTUSE_nb_reset_called" in kwargs:
+            self.__nb_reset_called = int(kwargs["_DONTUSE_nb_reset_called"])
+            del kwargs["_DONTUSE_nb_reset_called"]
+        else:
+            self.__nb_reset_called = -1
+        if "_DONTUSE_nb_step_called" in kwargs:
+            self.__nb_step_called = int(kwargs["_DONTUSE_nb_step_called"])
+            del kwargs["_DONTUSE_nb_step_called"]
+        else:
+            self.__nb_step_called = -1
+
+        if "_DONTUSE_nb_init_called" in kwargs:
+            self.__nb_init_called = int(kwargs["_DONTUSE_nb_init_called"])
+            del kwargs["_DONTUSE_nb_init_called"]
+        else:
+            self.__nb_init_called = -1
+
+        # now init the data
         Multifolder.__init__(
             self,
             path=path,
@@ -167,6 +194,7 @@ def reset(self):
         if self.cache_size == 0:
             raise RuntimeError("Impossible to initialize the new cache.")
 
+        self.__nb_reset_called += 1
         return self.subpaths[self._order]
 
     def initialize(
@@ -177,6 +205,12 @@ def initialize(
         order_backend_subs,
         names_chronics_to_backend=None,
     ):
+        self.__nb_init_called += 1
+        if self.__nb_reset_called <= 0:
+            if self.__nb_init_called != 0:
+                # authorize the creation of the environment but nothing more
+                raise ChronicsError(type(self).ERROR_MSG_NOT_LOADED)
+
         self._order_backend_loads = order_backend_loads
         self._order_backend_prods = order_backend_prods
         self._order_backend_lines = order_backend_lines
@@ -193,3 +227,23 @@ def initialize(
         id_scenario = self._order[self._prev_cache_id]
         self.data = self._cached_data[id_scenario]
         self.data.next_chronics()
+
+    def load_next(self):
+        self.__nb_step_called += 1
+        if self.__nb_reset_called <= 0:
+            if self.__nb_step_called != 0:
+                # authorize the creation of the environment but nothing more
+                raise ChronicsError(type(self).ERROR_MSG_NOT_LOADED)
+        return super().load_next()
+
+    def set_filter(self, filter_fun):
+        self.__nb_reset_called = 0
+        self.__nb_step_called = 0
+        self.__nb_init_called = 0
+        return super().set_filter(filter_fun)
+
+    def get_kwargs(self, dict_):
+        dict_["_DONTUSE_nb_reset_called"] = self.__nb_reset_called
+        dict_["_DONTUSE_nb_step_called"] = self.__nb_step_called
+        dict_["_DONTUSE_nb_init_called"] = self.__nb_init_called
+        return super().get_kwargs(dict_)

grid2op/tests/test_ChronicsHandler.py

@@ -1596,6 +1596,7 @@ def test_load(self):
                 chronics_class=MultifolderWithCache,
             ) as env:
                 env.seed(123456)  # for reproducible tests !
+                env.chronics_handler.reset()
                 nb_steps = 10
                 # I test that the reset ... reset also the date time and the chronics "state"
                 obs = env.reset()

grid2op/tests/test_feature_issue_447.py

@@ -0,0 +1,89 @@
+import grid2op
+from grid2op.Runner import Runner
+from grid2op.Reward import LinesCapacityReward  # or any other rewards
+from lightsim2grid import LightSimBackend  # highly recommended !
+from grid2op.Chronics import MultifolderWithCache  # highly recommended for training
+import warnings
+import unittest
+from grid2op.Exceptions import ChronicsError
+from grid2op.Runner import Runner
+
+
+class TestPreventWrongBehaviour(unittest.TestCase):
+    def setUp(self) -> None:
+        env_name = "l2rpn_case14_sandbox"
+        with warnings.catch_warnings():
+            warnings.filterwarnings("ignore")
+            self.env = grid2op.make(env_name,
+                                    reward_class=LinesCapacityReward,
+                                    backend=LightSimBackend(),
+                                    chronics_class=MultifolderWithCache,
+                                    test=True)
+    def tearDown(self) -> None:
+        self.env.close()
+
+    def test_can_make(self):
+        pass
+
+    def test_cannot_step(self):
+        with self.assertRaises(ChronicsError):
+            self.env.step(self.env.action_space())
+
+    def test_cannot_reset(self):
+        with self.assertRaises(ChronicsError):
+            obs = self.env.reset()
+
+    def test_can_reset(self):
+        self.env.chronics_handler.reset()
+        obs = self.env.reset()
+        self.env.step(self.env.action_space())
+
+    def test_can_reset(self):
+        self.env.chronics_handler.reset()
+        obs = self.env.reset()
+
+    def test_when_change_filter(self):
+        self.env.chronics_handler.set_filter(lambda x: True)
+        with self.assertRaises(ChronicsError):
+            obs = self.env.reset()
+        self.env.chronics_handler.reset()
+        obs = self.env.reset()
+        self.env.step(self.env.action_space())
+
+    def test_runner(self):
+        with self.assertRaises(ChronicsError):
+            runner = Runner(**self.env.get_params_for_runner())
+            res = runner.run(nb_episode=1,
+                            nb_process=1,
+                            max_iter=5
+                            )
+
+        self.env.chronics_handler.real_data.reset()
+        runner = Runner(**self.env.get_params_for_runner())
+        res = runner.run(nb_episode=1,
+                         nb_process=1,
+                         max_iter=5
+                        )
+
+    def test_copy(self):
+        # when the copied env is not init
+        env_cpy = self.env.copy()
+        with self.assertRaises(ChronicsError):
+            env_cpy.step(self.env.action_space())
+        with self.assertRaises(ChronicsError):
+            env_cpy.reset()
+        env_cpy.chronics_handler.reset()
+        obs = env_cpy.reset()
+        env_cpy.step(env_cpy.action_space())
+        env_cpy.close()
+
+        # if the copied env is properly init
+        self.env.chronics_handler.reset()
+        env_cpy2 = self.env.copy()
+        env_cpy2.chronics_handler.reset()
+        obs = env_cpy2.reset()
+        env_cpy2.step(env_cpy2.action_space())
+
+
+if __name__ == "__main__":
+    unittest.main()