From 7b7f6b3d39ba51c6cb4076ce5ebca72f49485d59 Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Fri, 8 Nov 2024 16:03:06 +0100 Subject: [PATCH 01/17] change: def file points to new sf repo --- resources/retinal-rl.def | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/retinal-rl.def b/resources/retinal-rl.def index 5f33858..cddf490 100644 --- a/resources/retinal-rl.def +++ b/resources/retinal-rl.def @@ -74,7 +74,7 @@ From: ubuntu:22.04 torchinfo==1.8.0 \ num2words==0.5.13 \ omgifol==0.5.1 \ - git+https://github.com/alex404/sample-factory.git@05465ef425530c3e2ef13dd1f9aa2d7313eb3d4a \ + git+https://github.com/fabioseel/sample-factory.git@abbc4591fcfa3f2cb20b98bb9b0f2a1ee83f47fa \ dpcpp-cpp-rt==2024.2.1 \ seaborn==0.13.2 \ hydra-core==1.3.2 \ From 6f24fda35698ffae3df0f0c41ed349ae31b64708 Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Fri, 8 Nov 2024 16:03:55 +0100 Subject: [PATCH 02/17] doc: update doc in compile scenario --- doom_creator/compile_scenario.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doom_creator/compile_scenario.py b/doom_creator/compile_scenario.py index d6de3eb..7fb5dbf 100644 --- a/doom_creator/compile_scenario.py +++ b/doom_creator/compile_scenario.py @@ -41,7 +41,7 @@ def make_parser(): running the first time one should use the --preload flag to download the necessary resources into the --out_dir ('{Directories().CACHE_DIR}'). """, - epilog="Example: python -m exec.compile_scenario gathering apples", + epilog="Example: python -m doom_creator.compile_scenario gathering apples", ) # Positional argument for scenario yaml files (required, can be multiple) parser.add_argument( From 9128475b864f6917c30678518e8e10cec9040e6e Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Fri, 8 Nov 2024 16:05:16 +0100 Subject: [PATCH 03/17] chore: make to_sf_cfg static --- runner/frameworks/rl/sf_framework.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/runner/frameworks/rl/sf_framework.py b/runner/frameworks/rl/sf_framework.py index 601fc74..f5f5062 100644 --- a/runner/frameworks/rl/sf_framework.py +++ b/runner/frameworks/rl/sf_framework.py @@ -113,25 +113,26 @@ def load_brain_and_config( brain.to(device) return brain - def to_sf_cfg(self, cfg: DictConfig) -> Config: - sf_cfg = self._get_default_cfg(cfg.dataset.env_name) # Load Defaults + @staticmethod + def to_sf_cfg(cfg: DictConfig) -> Config: + sf_cfg = SFFramework._get_default_cfg(cfg.dataset.env_name) # Load Defaults # overwrite default values with those set in cfg # TODO: which other parameters need to be set_ - self._set_cfg_cli_argument(sf_cfg, "learning_rate", cfg.optimizer.optimizer.lr) + SFFramework._set_cfg_cli_argument(sf_cfg, "learning_rate", cfg.optimizer.optimizer.lr) # Using this function is necessary to make sure that the parameters are not overwritten when sample_factory loads a checkpoint - self._set_cfg_cli_argument(sf_cfg, "res_h", cfg.dataset.vision_width) - self._set_cfg_cli_argument(sf_cfg, "res_w", cfg.dataset.vision_height) - self._set_cfg_cli_argument(sf_cfg, "env", cfg.dataset.env_name) - self._set_cfg_cli_argument(sf_cfg, "input_satiety", cfg.dataset.input_satiety) - self._set_cfg_cli_argument(sf_cfg, "device", cfg.system.device) + SFFramework._set_cfg_cli_argument(sf_cfg, "res_h", cfg.dataset.vision_width) + SFFramework._set_cfg_cli_argument(sf_cfg, "res_w", cfg.dataset.vision_height) + SFFramework._set_cfg_cli_argument(sf_cfg, "env", cfg.dataset.env_name) + SFFramework._set_cfg_cli_argument(sf_cfg, "input_satiety", cfg.dataset.input_satiety) + SFFramework._set_cfg_cli_argument(sf_cfg, "device", cfg.system.device) optimizer_name = str.lower( str.split(cfg.optimizer.optimizer._target_, sep=".")[-1] ) - self._set_cfg_cli_argument(sf_cfg, "optimizer", optimizer_name) + SFFramework._set_cfg_cli_argument(sf_cfg, "optimizer", optimizer_name) - self._set_cfg_cli_argument(sf_cfg, "brain", OmegaConf.to_object(cfg.brain)) + SFFramework._set_cfg_cli_argument(sf_cfg, "brain", OmegaConf.to_object(cfg.brain)) return sf_cfg def analyze( From 6660db62ed8dd35c8c019ac96a41fe265b8ca8f6 Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Fri, 15 Nov 2024 09:48:39 +0100 Subject: [PATCH 04/17] hotfix: make rl config without objective set runnable --- main.py | 1 + 1 file changed, 1 insertion(+) diff --git a/main.py b/main.py index c2351ea..b9d205d 100644 --- a/main.py +++ b/main.py @@ -44,6 +44,7 @@ def _program(cfg: DictConfig): objective = instantiate(cfg.optimizer.objective, brain=brain) # TODO: RL framework currently can't use objective else: + objective=None warnings.warn("No objective specified, is that wanted?") if cfg.command == "scan": From 932eeedb40d2d26510d75ae20d80b4f38ca9f031 Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Fri, 15 Nov 2024 09:50:32 +0100 Subject: [PATCH 05/17] feat: set train & wandb dirs for sample factory --- runner/frameworks/rl/sf_framework.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/runner/frameworks/rl/sf_framework.py b/runner/frameworks/rl/sf_framework.py index f5f5062..6a479cc 100644 --- a/runner/frameworks/rl/sf_framework.py +++ b/runner/frameworks/rl/sf_framework.py @@ -61,7 +61,7 @@ def train( objective: Optional[Objective[ContextT]] = None, ): warnings.warn( - "device, brain, optimizer are initialized differently in sample_factory and thus there current state will be ignored" + "device, brain, optimizer are initialized differently in sample_factory and thus their current state will be ignored" ) warnings.warn( "objective is currently not supported for sample factory simulations" @@ -133,6 +133,8 @@ def to_sf_cfg(cfg: DictConfig) -> Config: SFFramework._set_cfg_cli_argument(sf_cfg, "optimizer", optimizer_name) SFFramework._set_cfg_cli_argument(sf_cfg, "brain", OmegaConf.to_object(cfg.brain)) + SFFramework._set_cfg_cli_argument(sf_cfg, "train_dir", os.path.join(cfg.path.run_dir, "train_dir")) + SFFramework._set_cfg_cli_argument(sf_cfg, "wandb_dir", cfg.path.wandb_dir) return sf_cfg def analyze( @@ -142,7 +144,7 @@ def analyze( objective: Optional[Objective[ContextT]] = None, ): warnings.warn( - "device, brain, optimizer are initialized differently in sample_factory and thus there current state will be ignored" + "device, brain, optimizer are initialized differently in sample_factory and thus their current state will be ignored" ) enjoy(self.sf_cfg) # TODO: Implement analyze function for sf framework From 650ba69a59b9986b10706c57d3275d63d233b076 Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Fri, 15 Nov 2024 09:53:28 +0100 Subject: [PATCH 06/17] feat: add simple tests for sample_factory with pytest --- resources/retinal-rl.def | 4 ++- runner/util.py | 26 ++++++++++++++++++ tests/ci/build_scenario.sh | 13 +++++++++ tests/modules/conftest.py | 41 ++++++++++++++++++++++++++++ tests/modules/test_sample_factory.py | 32 ++++++++++++++++++++++ 5 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 tests/ci/build_scenario.sh create mode 100644 tests/modules/conftest.py create mode 100644 tests/modules/test_sample_factory.py diff --git a/resources/retinal-rl.def b/resources/retinal-rl.def index cddf490..2035f18 100644 --- a/resources/retinal-rl.def +++ b/resources/retinal-rl.def @@ -70,16 +70,18 @@ From: ubuntu:22.04 opencv-python==4.10.0.84 \ pygame==2.6.1 \ pycairo==1.26.1 \ + pytest==8.3.3 \ git+https://github.com/pytorch/captum.git@fd758e025673100cb6a525d59a78893c558b825b \ torchinfo==1.8.0 \ num2words==0.5.13 \ omgifol==0.5.1 \ - git+https://github.com/fabioseel/sample-factory.git@abbc4591fcfa3f2cb20b98bb9b0f2a1ee83f47fa \ dpcpp-cpp-rt==2024.2.1 \ seaborn==0.13.2 \ hydra-core==1.3.2 \ networkx==3.3 \ ruff==0.7.0 + + pip3 install -e ../sample-factory # Clean up for smaller container size rm acc159linux-x64.zip diff --git a/runner/util.py b/runner/util.py index fbfea1b..580d331 100644 --- a/runner/util.py +++ b/runner/util.py @@ -196,3 +196,29 @@ def _resolve_output_shape( raise ValueError( f"Invalid format for output_shape: {output_shape}. Must be of the form 'circuit_name.property_name'" ) + +def search_conf(config: DictConfig | dict, search_str: str) -> List: + """ + Recursively search for strings in a DictConfig. + + Args: + config (omegaconf.DictConfig): The configuration to search. + + Returns: + list: A list of all values containing the string. + """ + found_values = [] + + def traverse_config(cfg): + for key, value in cfg.items(): + if isinstance(value, (dict, DictConfig)): + traverse_config(value) + elif isinstance(value, str) and search_str in value: + found_values.append(value) + elif isinstance(value, list): + for item in value: + if isinstance(item, str) and search_str in item: + found_values.append(item) + + traverse_config(config) + return found_values diff --git a/tests/ci/build_scenario.sh b/tests/ci/build_scenario.sh new file mode 100644 index 0000000..da20d50 --- /dev/null +++ b/tests/ci/build_scenario.sh @@ -0,0 +1,13 @@ +#!/bin/bash +#=============================================================================== +# Description: Builds the gathering apples scenario used for tests +# +# Arguments: +# $1 - Path to Singularity (.sif) container +# +# Usage: +# tests/ci/build_scenario.sh container.sif +# (run from top level directory!) +#=============================================================================== + +apptainer exec "$CONTAINER" python -m doom_creator.compile_scenario gathering apples \ No newline at end of file diff --git a/tests/modules/conftest.py b/tests/modules/conftest.py new file mode 100644 index 0000000..b11aad0 --- /dev/null +++ b/tests/modules/conftest.py @@ -0,0 +1,41 @@ +import os +import shutil +import sys +import time + +import hydra +import pytest +from omegaconf import DictConfig, OmegaConf + +sys.path.append(".") +from runner.util import search_conf + +OmegaConf.register_new_resolver("eval", eval) + + +@pytest.fixture +def config() -> DictConfig: + with hydra.initialize(config_path="../../config/base", version_base=None): + experiment = "gathering-apples" + config = hydra.compose("config", overrides=[f"+experiment={experiment}", "system.device=cpu"]) + + # replace the paths that are normally set via HydraConfig + config.path.run_dir=f"tmp{hash(time.time())}" + config.sweep.command[-2]=experiment + + # check whether there's still values to be interpolated through hydra + hydra_values = search_conf(OmegaConf.to_container(config, resolve=False), "hydra:") + assert len(hydra_values) == 0, "hydra: values can not be resolved here. Set them manually in this fixture for tests!" + + + OmegaConf.resolve(config) + yield config + + # Cleanup: remove temporary dir + if os.path.exists(config.path.run_dir): + shutil.rmtree(config.path.run_dir) + + +@pytest.fixture +def data_root() -> str: + return "cache" diff --git a/tests/modules/test_sample_factory.py b/tests/modules/test_sample_factory.py new file mode 100644 index 0000000..a986037 --- /dev/null +++ b/tests/modules/test_sample_factory.py @@ -0,0 +1,32 @@ +import sys + +from omegaconf import DictConfig, OmegaConf +from sample_factory.algo.utils.context import global_model_factory +from sample_factory.algo.utils.make_env import make_env_func_batched +from sample_factory.algo.utils.misc import ExperimentStatus +from sample_factory.model.actor_critic import create_actor_critic +from sample_factory.train import make_runner +from sample_factory.utils.attr_dict import AttrDict + +sys.path.append(".") +from retinal_rl.rl.sample_factory.environment import register_retinal_env +from retinal_rl.rl.sample_factory.models import SampleFactoryBrain +from runner.frameworks.rl.sf_framework import SFFramework + + +def test_init_framework(config: DictConfig, data_root): + framework = SFFramework(config, data_root) + cfg, runner = make_runner(framework.sf_cfg) + status = runner.init() + assert status==ExperimentStatus.SUCCESS + + +def test_actor_critic_brain(config: DictConfig, data_root): + sf_cfg = SFFramework.to_sf_cfg(config) + register_retinal_env(sf_cfg.env, data_root, False) + global_model_factory().register_actor_critic_factory(SampleFactoryBrain) + env = make_env_func_batched( + sf_cfg, env_config=AttrDict(worker_index=0, vector_index=0, env_id=0) + ) + + create_actor_critic(sf_cfg, env.observation_space, env.action_space) From 0e814454b44471a1b999016bdb4bf5f480aedd40 Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Fri, 15 Nov 2024 10:04:32 +0100 Subject: [PATCH 07/17] fix: relative path for configs should be relative to module --- retinal_rl/rl/sample_factory/environment.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/retinal_rl/rl/sample_factory/environment.py b/retinal_rl/rl/sample_factory/environment.py index bf0087b..4a970a2 100644 --- a/retinal_rl/rl/sample_factory/environment.py +++ b/retinal_rl/rl/sample_factory/environment.py @@ -10,6 +10,8 @@ from sample_factory.envs.env_utils import register_env from sf_examples.vizdoom.doom.doom_utils import DoomSpec, make_doom_env_impl +import retinal_rl + # from gym.spaces import Discrete @@ -131,9 +133,12 @@ def make_retinal_env_from_spec( def register_retinal_env(scene_name: str, cache_dir: str, input_satiety: bool): - cfg_path = os.path.join( - cache_dir, "scenarios", scene_name + ".cfg" - ) # TODO: Check if this stays + print(cache_dir) + if not os.path.isabs(cache_dir): + # make path absolute by making it relative to the path of this file + # TODO: Discuss whether this is desired behaviour... + cache_dir = os.path.join(os.path.dirname(__file__), "..", "..", "..", cache_dir) + cfg_path = os.path.join(cache_dir, "scenarios", scene_name + ".cfg") env_spec = retinal_doomspec(scene_name, cfg_path, input_satiety) make_env_func = functools.partial(make_retinal_env_from_spec, env_spec) From d59f12ae66c4a8ac2dfee72b0c58894a99d59300 Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Tue, 19 Nov 2024 12:10:00 +0100 Subject: [PATCH 08/17] feat: add pytest and test to workflow --- .github/workflows/code_check.yml | 4 ++++ resources/retinal-rl.def | 3 +-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/code_check.yml b/.github/workflows/code_check.yml index 8833064..b06ad9d 100644 --- a/.github/workflows/code_check.yml +++ b/.github/workflows/code_check.yml @@ -40,3 +40,7 @@ jobs: run: | bash tests/ci/copy_configs.sh ${{ env.sif_file }} bash tests/ci/scan_configs.sh ${{ env.sif_file }} + + - name: Pytest + if: always() && steps.cache-singularity.outputs.cache-hit == 'true' + run: apptainer exec ${{ env.sif_file }} pytest tests/modules \ No newline at end of file diff --git a/resources/retinal-rl.def b/resources/retinal-rl.def index 2035f18..497e0d1 100644 --- a/resources/retinal-rl.def +++ b/resources/retinal-rl.def @@ -75,13 +75,12 @@ From: ubuntu:22.04 torchinfo==1.8.0 \ num2words==0.5.13 \ omgifol==0.5.1 \ + git+https://github.com/fabioseel/sample-factory.git@abbc4591fcfa3f2cb20b98bb9b0f2a1ee83f47fa \ dpcpp-cpp-rt==2024.2.1 \ seaborn==0.13.2 \ hydra-core==1.3.2 \ networkx==3.3 \ ruff==0.7.0 - - pip3 install -e ../sample-factory # Clean up for smaller container size rm acc159linux-x64.zip From 53230c32e34f4d4f52d45caefeef613e8d63207b Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Wed, 20 Nov 2024 11:04:54 +0100 Subject: [PATCH 09/17] fix: workflow for rl tests needs to build scenario first --- .github/workflows/code_check.yml | 4 +++- tests/ci/build_scenario.sh | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/code_check.yml b/.github/workflows/code_check.yml index b06ad9d..b5032a0 100644 --- a/.github/workflows/code_check.yml +++ b/.github/workflows/code_check.yml @@ -43,4 +43,6 @@ jobs: - name: Pytest if: always() && steps.cache-singularity.outputs.cache-hit == 'true' - run: apptainer exec ${{ env.sif_file }} pytest tests/modules \ No newline at end of file + run: | + bash tests/ci/build_scenario.sh ${{ env.sif_file }} + apptainer exec ${{ env.sif_file }} pytest tests/modules \ No newline at end of file diff --git a/tests/ci/build_scenario.sh b/tests/ci/build_scenario.sh index da20d50..478706a 100644 --- a/tests/ci/build_scenario.sh +++ b/tests/ci/build_scenario.sh @@ -10,4 +10,4 @@ # (run from top level directory!) #=============================================================================== -apptainer exec "$CONTAINER" python -m doom_creator.compile_scenario gathering apples \ No newline at end of file +apptainer exec "$1" python -m doom_creator.compile_scenario gathering apples \ No newline at end of file From 0448df27f58bdb9aa31a09aa5f5685fd47ba707e Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Wed, 20 Nov 2024 11:12:18 +0100 Subject: [PATCH 10/17] fix: linting issues --- main.py | 2 +- retinal_rl/rl/sample_factory/environment.py | 2 -- runner/frameworks/rl/sf_framework.py | 16 ++++++++++++---- runner/util.py | 1 + tests/modules/conftest.py | 16 +++++++++++----- tests/modules/test_sample_factory.py | 4 ++-- 6 files changed, 27 insertions(+), 14 deletions(-) diff --git a/main.py b/main.py index b9d205d..a0b706d 100644 --- a/main.py +++ b/main.py @@ -44,7 +44,7 @@ def _program(cfg: DictConfig): objective = instantiate(cfg.optimizer.objective, brain=brain) # TODO: RL framework currently can't use objective else: - objective=None + objective = None warnings.warn("No objective specified, is that wanted?") if cfg.command == "scan": diff --git a/retinal_rl/rl/sample_factory/environment.py b/retinal_rl/rl/sample_factory/environment.py index 4a970a2..c0c1e3c 100644 --- a/retinal_rl/rl/sample_factory/environment.py +++ b/retinal_rl/rl/sample_factory/environment.py @@ -10,8 +10,6 @@ from sample_factory.envs.env_utils import register_env from sf_examples.vizdoom.doom.doom_utils import DoomSpec, make_doom_env_impl -import retinal_rl - # from gym.spaces import Discrete diff --git a/runner/frameworks/rl/sf_framework.py b/runner/frameworks/rl/sf_framework.py index 6a479cc..4b071f0 100644 --- a/runner/frameworks/rl/sf_framework.py +++ b/runner/frameworks/rl/sf_framework.py @@ -119,21 +119,29 @@ def to_sf_cfg(cfg: DictConfig) -> Config: # overwrite default values with those set in cfg # TODO: which other parameters need to be set_ - SFFramework._set_cfg_cli_argument(sf_cfg, "learning_rate", cfg.optimizer.optimizer.lr) + SFFramework._set_cfg_cli_argument( + sf_cfg, "learning_rate", cfg.optimizer.optimizer.lr + ) # Using this function is necessary to make sure that the parameters are not overwritten when sample_factory loads a checkpoint SFFramework._set_cfg_cli_argument(sf_cfg, "res_h", cfg.dataset.vision_width) SFFramework._set_cfg_cli_argument(sf_cfg, "res_w", cfg.dataset.vision_height) SFFramework._set_cfg_cli_argument(sf_cfg, "env", cfg.dataset.env_name) - SFFramework._set_cfg_cli_argument(sf_cfg, "input_satiety", cfg.dataset.input_satiety) + SFFramework._set_cfg_cli_argument( + sf_cfg, "input_satiety", cfg.dataset.input_satiety + ) SFFramework._set_cfg_cli_argument(sf_cfg, "device", cfg.system.device) optimizer_name = str.lower( str.split(cfg.optimizer.optimizer._target_, sep=".")[-1] ) SFFramework._set_cfg_cli_argument(sf_cfg, "optimizer", optimizer_name) - SFFramework._set_cfg_cli_argument(sf_cfg, "brain", OmegaConf.to_object(cfg.brain)) - SFFramework._set_cfg_cli_argument(sf_cfg, "train_dir", os.path.join(cfg.path.run_dir, "train_dir")) + SFFramework._set_cfg_cli_argument( + sf_cfg, "brain", OmegaConf.to_object(cfg.brain) + ) + SFFramework._set_cfg_cli_argument( + sf_cfg, "train_dir", os.path.join(cfg.path.run_dir, "train_dir") + ) SFFramework._set_cfg_cli_argument(sf_cfg, "wandb_dir", cfg.path.wandb_dir) return sf_cfg diff --git a/runner/util.py b/runner/util.py index 580d331..ed1fdcb 100644 --- a/runner/util.py +++ b/runner/util.py @@ -197,6 +197,7 @@ def _resolve_output_shape( f"Invalid format for output_shape: {output_shape}. Must be of the form 'circuit_name.property_name'" ) + def search_conf(config: DictConfig | dict, search_str: str) -> List: """ Recursively search for strings in a DictConfig. diff --git a/tests/modules/conftest.py b/tests/modules/conftest.py index b11aad0..800b225 100644 --- a/tests/modules/conftest.py +++ b/tests/modules/conftest.py @@ -17,16 +17,22 @@ def config() -> DictConfig: with hydra.initialize(config_path="../../config/base", version_base=None): experiment = "gathering-apples" - config = hydra.compose("config", overrides=[f"+experiment={experiment}", "system.device=cpu"]) + config = hydra.compose( + "config", overrides=[f"+experiment={experiment}", "system.device=cpu"] + ) # replace the paths that are normally set via HydraConfig - config.path.run_dir=f"tmp{hash(time.time())}" - config.sweep.command[-2]=experiment + config.path.run_dir = f"tmp{hash(time.time())}" + config.sweep.command[-2] = experiment # check whether there's still values to be interpolated through hydra - hydra_values = search_conf(OmegaConf.to_container(config, resolve=False), "hydra:") - assert len(hydra_values) == 0, "hydra: values can not be resolved here. Set them manually in this fixture for tests!" + hydra_values = search_conf( + OmegaConf.to_container(config, resolve=False), "hydra:" + ) + assert ( + len(hydra_values) == 0 + ), "hydra: values can not be resolved here. Set them manually in this fixture for tests!" OmegaConf.resolve(config) yield config diff --git a/tests/modules/test_sample_factory.py b/tests/modules/test_sample_factory.py index a986037..0c1b3e2 100644 --- a/tests/modules/test_sample_factory.py +++ b/tests/modules/test_sample_factory.py @@ -1,6 +1,6 @@ import sys -from omegaconf import DictConfig, OmegaConf +from omegaconf import DictConfig from sample_factory.algo.utils.context import global_model_factory from sample_factory.algo.utils.make_env import make_env_func_batched from sample_factory.algo.utils.misc import ExperimentStatus @@ -18,7 +18,7 @@ def test_init_framework(config: DictConfig, data_root): framework = SFFramework(config, data_root) cfg, runner = make_runner(framework.sf_cfg) status = runner.init() - assert status==ExperimentStatus.SUCCESS + assert status == ExperimentStatus.SUCCESS def test_actor_critic_brain(config: DictConfig, data_root): From 6b4bb58c900d2fe4044e19e1e93a73690eb1db98 Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Wed, 20 Nov 2024 11:26:50 +0100 Subject: [PATCH 11/17] fix: ensure out dir for scenario creation exists --- doom_creator/compile_scenario.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doom_creator/compile_scenario.py b/doom_creator/compile_scenario.py index 7fb5dbf..97247de 100644 --- a/doom_creator/compile_scenario.py +++ b/doom_creator/compile_scenario.py @@ -14,6 +14,7 @@ import os import sys import warnings +from pathlib import Path from doom_creator.util.config import load from doom_creator.util.directories import Directories @@ -106,6 +107,9 @@ def main(): args = parser.parse_args(argv) dirs = Directories(args.out_dir) + + Path(dirs.CACHE_DIR).mkdir(parents=True, exist_ok=True) + cfg = load(args.yamls, dirs.SCENARIO_YAML_DIR) # Check preload flag do_load, do_make, do_list = args.preload, len(args.yamls) > 0, args.list_yamls From 4a6249f7b5e300df1428c36718e2f5aea27f38a2 Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Wed, 20 Nov 2024 11:35:43 +0100 Subject: [PATCH 12/17] fix: pass wandb usage option to sample_factory --- doom_creator/util/preload.py | 6 +++--- runner/frameworks/rl/sf_framework.py | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/doom_creator/util/preload.py b/doom_creator/util/preload.py index 6292e03..8c24bc4 100644 --- a/doom_creator/util/preload.py +++ b/doom_creator/util/preload.py @@ -3,7 +3,7 @@ import shutil import struct from glob import glob -from typing import Optional +from typing import Optional, Set, Tuple from PIL import Image from PIL.PngImagePlugin import PngInfo @@ -105,8 +105,8 @@ def preload_dataset( dataset_wrapper.clean(source_dir) -def check_preload(cfg: Config, test: bool): - needed_types = set() +def check_preload(cfg: Config, test: bool) -> Tuple[Config, Set[TextureType]]: + needed_types: Set[TextureType] = set() for type_cfg in cfg.objects.values(): for actor in type_cfg.actors.values(): for i in range(len(actor.textures)): diff --git a/runner/frameworks/rl/sf_framework.py b/runner/frameworks/rl/sf_framework.py index 4b071f0..b794213 100644 --- a/runner/frameworks/rl/sf_framework.py +++ b/runner/frameworks/rl/sf_framework.py @@ -142,6 +142,7 @@ def to_sf_cfg(cfg: DictConfig) -> Config: SFFramework._set_cfg_cli_argument( sf_cfg, "train_dir", os.path.join(cfg.path.run_dir, "train_dir") ) + SFFramework._set_cfg_cli_argument(sf_cfg, "with_wandb", cfg.logging.use_wandb) SFFramework._set_cfg_cli_argument(sf_cfg, "wandb_dir", cfg.path.wandb_dir) return sf_cfg From be5b12b69f7cfe5add6520f7648f50dc1a85e288 Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Wed, 20 Nov 2024 12:00:26 +0100 Subject: [PATCH 13/17] fix: ensure scenario dir exists. partial change to pathlib --- doom_creator/compile_scenario.py | 3 --- doom_creator/util/directories.py | 44 ++++++++++++++++---------------- doom_creator/util/make.py | 1 + tests/ci/build_scenario.sh | 0 4 files changed, 23 insertions(+), 25 deletions(-) mode change 100644 => 100755 tests/ci/build_scenario.sh diff --git a/doom_creator/compile_scenario.py b/doom_creator/compile_scenario.py index 97247de..fe8707b 100644 --- a/doom_creator/compile_scenario.py +++ b/doom_creator/compile_scenario.py @@ -14,7 +14,6 @@ import os import sys import warnings -from pathlib import Path from doom_creator.util.config import load from doom_creator.util.directories import Directories @@ -108,8 +107,6 @@ def main(): dirs = Directories(args.out_dir) - Path(dirs.CACHE_DIR).mkdir(parents=True, exist_ok=True) - cfg = load(args.yamls, dirs.SCENARIO_YAML_DIR) # Check preload flag do_load, do_make, do_list = args.preload, len(args.yamls) > 0, args.list_yamls diff --git a/doom_creator/util/directories.py b/doom_creator/util/directories.py index 8973622..97fc52d 100644 --- a/doom_creator/util/directories.py +++ b/doom_creator/util/directories.py @@ -1,47 +1,47 @@ -import os.path as osp from dataclasses import dataclass from typing import Optional +from pathlib import Path @dataclass class Directories: - cache_dir: str = "cache" - resource_dir: str = osp.join("doom_creator", "resources") # noqa: RUF009 as this is not really a dynamical call - scenario_out_dir: Optional[str] = None - build_dir: Optional[str] = None - textures_dir: Optional[str] = None - assets_dir: Optional[str] = None - scenario_yaml_dir: Optional[str] = None - dataset_dir: Optional[str] = None + cache_dir: Path = "cache" + resource_dir: Path = Path("doom_creator", "resources") + scenario_out_dir: Optional[Path] = None + build_dir: Optional[Path] = None + textures_dir: Optional[Path] = None + assets_dir: Optional[Path] = None + scenario_yaml_dir: Optional[Path] = None + dataset_dir: Optional[Path] = None def __post_init__(self): - self.CACHE_DIR = self.cache_dir - self.SCENARIO_OUT_DIR = ( - osp.join(self.CACHE_DIR, "scenarios") + self.CACHE_DIR: Path = self.cache_dir + self.SCENARIO_OUT_DIR: Path = ( + Path(self.CACHE_DIR, "scenarios") if self.scenario_out_dir is None else self.scenario_out_dir ) - self.BUILD_DIR = ( - osp.join(self.SCENARIO_OUT_DIR, "build") + self.BUILD_DIR: Path = ( + Path(self.SCENARIO_OUT_DIR, "build") if self.build_dir is None else self.build_dir ) - self.TEXTURES_DIR = ( - osp.join(self.CACHE_DIR, "textures") + self.TEXTURES_DIR: Path = ( + Path(self.CACHE_DIR, "textures") if self.textures_dir is None else self.textures_dir ) - self.RESOURCE_DIR = self.resource_dir - self.ASSETS_DIR = ( - osp.join(self.RESOURCE_DIR, "assets") + self.RESOURCE_DIR: Path = self.resource_dir + self.ASSETS_DIR: Path = ( + Path(self.RESOURCE_DIR, "assets") if self.assets_dir is None else self.assets_dir ) - self.SCENARIO_YAML_DIR = ( - osp.join(self.RESOURCE_DIR, "config") + self.SCENARIO_YAML_DIR: Path = ( + Path(self.RESOURCE_DIR, "config") if self.scenario_yaml_dir is None else self.scenario_yaml_dir ) - self.DATASET_DIR = ( + self.DATASET_DIR: Path = ( self.TEXTURES_DIR if self.dataset_dir is None else self.dataset_dir ) diff --git a/doom_creator/util/make.py b/doom_creator/util/make.py index eb3f129..cb7ae61 100644 --- a/doom_creator/util/make.py +++ b/doom_creator/util/make.py @@ -19,6 +19,7 @@ def make_scenario( scenario_name: Optional[str] = None, ): # Create Zip for output + directories.SCENARIO_OUT_DIR.mkdir(parents=True, exist_ok=True) out_file = osp.join(directories.SCENARIO_OUT_DIR, scenario_name) + ".zip" if osp.exists(out_file): os.remove(out_file) diff --git a/tests/ci/build_scenario.sh b/tests/ci/build_scenario.sh old mode 100644 new mode 100755 From a87b6e8c9f098ffec0d709935bb8cb45300b500a Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Wed, 20 Nov 2024 13:08:31 +0100 Subject: [PATCH 14/17] fix: lint issues, make code_check more expressive --- .github/workflows/code_check.yml | 8 +++++--- doom_creator/util/directories.py | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/code_check.yml b/.github/workflows/code_check.yml index b5032a0..b7919e9 100644 --- a/.github/workflows/code_check.yml +++ b/.github/workflows/code_check.yml @@ -40,9 +40,11 @@ jobs: run: | bash tests/ci/copy_configs.sh ${{ env.sif_file }} bash tests/ci/scan_configs.sh ${{ env.sif_file }} + + - name: Build Scenario + if: always() && steps.cache-singularity.outputs.cache-hit == 'true' + run: bash tests/ci/build_scenario.sh ${{ env.sif_file }} - name: Pytest if: always() && steps.cache-singularity.outputs.cache-hit == 'true' - run: | - bash tests/ci/build_scenario.sh ${{ env.sif_file }} - apptainer exec ${{ env.sif_file }} pytest tests/modules \ No newline at end of file + run: apptainer exec ${{ env.sif_file }} pytest tests/modules \ No newline at end of file diff --git a/doom_creator/util/directories.py b/doom_creator/util/directories.py index 97fc52d..487eaca 100644 --- a/doom_creator/util/directories.py +++ b/doom_creator/util/directories.py @@ -1,6 +1,6 @@ from dataclasses import dataclass -from typing import Optional from pathlib import Path +from typing import Optional @dataclass From 3919e67c262406878319c265bd0054ec0ddb5b5e Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Wed, 20 Nov 2024 14:30:58 +0100 Subject: [PATCH 15/17] chore: change sample factory reference to (currently) newest version in official repo --- resources/retinal-rl.def | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/retinal-rl.def b/resources/retinal-rl.def index 497e0d1..c8a8f33 100644 --- a/resources/retinal-rl.def +++ b/resources/retinal-rl.def @@ -75,7 +75,7 @@ From: ubuntu:22.04 torchinfo==1.8.0 \ num2words==0.5.13 \ omgifol==0.5.1 \ - git+https://github.com/fabioseel/sample-factory.git@abbc4591fcfa3f2cb20b98bb9b0f2a1ee83f47fa \ + git+https://github.com/alex-petrenko/sample-factory.git@e9589e4218e838d8a1377380c27ad45feb44f4ba \ dpcpp-cpp-rt==2024.2.1 \ seaborn==0.13.2 \ hydra-core==1.3.2 \ From 302f7c7d1bb9dac7b102858310441cc51a26371c Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Fri, 22 Nov 2024 10:11:14 +0100 Subject: [PATCH 16/17] fix: actor_critic interface changed --- retinal_rl/rl/sample_factory/models.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/retinal_rl/rl/sample_factory/models.py b/retinal_rl/rl/sample_factory/models.py index 9933dec..3c2ab0d 100644 --- a/retinal_rl/rl/sample_factory/models.py +++ b/retinal_rl/rl/sample_factory/models.py @@ -1,6 +1,6 @@ import warnings from enum import Enum -from typing import Dict, Tuple +from typing import Dict, Optional, Tuple import networkx as nx import numpy as np @@ -116,7 +116,7 @@ def forward_core(self, head_output, rnn_states): return out, rnn_states def forward_tail( - self, core_output, values_only: bool, sample_actions: bool + self, core_output, values_only: bool, sample_actions: bool, action_mask: Optional[Tensor] = None ) -> TensorDict: out = self.brain.circuits[self.decoder_name](core_output) out = torch.flatten(out, 1) @@ -138,7 +138,7 @@ def forward_tail( return result def forward( - self, normalized_obs_dict, rnn_states, values_only: bool = False + self, normalized_obs_dict, rnn_states, values_only: bool = False, action_mask: Optional[Tensor] = None ) -> TensorDict: head_out = self.forward_head(normalized_obs_dict) core_out, new_rnn_states = self.forward_core(head_out, rnn_states) From e9d62f7fe6f738cc6aa273d8303026dda036b4dc Mon Sep 17 00:00:00 2001 From: Fabio Seel Date: Fri, 22 Nov 2024 10:20:21 +0100 Subject: [PATCH 17/17] fix: change command in scripts to singularity (if apptainer is installed, it sets up an alias) --- tests/ci/build_scenario.sh | 2 +- tests/ci/lint.sh | 4 ++-- tests/ci/scan_configs.sh | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/ci/build_scenario.sh b/tests/ci/build_scenario.sh index 478706a..4a982f0 100755 --- a/tests/ci/build_scenario.sh +++ b/tests/ci/build_scenario.sh @@ -10,4 +10,4 @@ # (run from top level directory!) #=============================================================================== -apptainer exec "$1" python -m doom_creator.compile_scenario gathering apples \ No newline at end of file +singularity exec "$1" python -m doom_creator.compile_scenario gathering apples diff --git a/tests/ci/lint.sh b/tests/ci/lint.sh index eb91638..093f3a8 100755 --- a/tests/ci/lint.sh +++ b/tests/ci/lint.sh @@ -43,9 +43,9 @@ fi if [ -n "$changed_files" ]; then # Format - apptainer exec "$CONTAINER" ruff format $changed_files $check + singularity exec "$CONTAINER" ruff format $changed_files $check # Run ruff on changed files with any remaining arguments - apptainer exec "$CONTAINER" ruff check $changed_files "$@" + singularity exec "$CONTAINER" ruff check $changed_files "$@" else echo "No .py files changed" fi diff --git a/tests/ci/scan_configs.sh b/tests/ci/scan_configs.sh index 2d08c58..e36aec5 100755 --- a/tests/ci/scan_configs.sh +++ b/tests/ci/scan_configs.sh @@ -17,6 +17,6 @@ for file in config/user/experiment/*.yaml; do experiment=$(basename "$file" .yaml) - apptainer exec "$1" \ + singularity exec "$1" \ python main.py +experiment="$experiment" command=scan system.device=cpu done \ No newline at end of file