Add pickle tests #53

elliottower · 2023-03-28T23:49:18Z

Description

This PR adds tests to ensure that each environment type can be saved and loaded via pickling. It adds EzPickle support to those environments which cannot be pickled, such as OpenSpiel.

Note: I made this as a fork of the previous PR adding dockerfile/CI support, could revert the commit and make a new branch but figured this was fine as that was merged.

Fixes # (issue), Depends on # (pull request)

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Screenshots

Please attach before and after screenshots of the change if applicable.
To upload images to a PR -- simply drag and drop or copy paste.

Checklist:

I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
I have run pytest -v and no errors are present.
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I solved any possible warnings that pytest -v has generated that are related to my code to the best of my knowledge.
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

# Conflicts: # .github/workflows/optional-install-test.yml

…enspiel/bsuite

pseudo-rnd-thoughts

Thanks Elliot for doing this, looks impressive as far

pseudo-rnd-thoughts · 2023-03-29T22:38:37Z

tests/test_bsuite.py

+# Without EzPickle:_register_bsuite_envs.<locals>._make_bsuite_env cannot be pickled
+# With EzPickle: maximum recursion limit reached
+FAILING_PICKLE_ENVS = [
+    "bsuite/bandit_noise-v0",


I think I have found this before. It is generally because one of the classes implemented __getattr__.
Is there a particular parameter that is missing?

I suspect the issue is that the environment only defines a variable on reset or another function

I’ll go check that individual env and see. But what would we be able to do to fix it, besides submitting a PR to their repo? I guess we could in the compatibility wrapper specifically check if it’s that env or if an env has that specific variable not defined in init, and then do whatever modifications are required?

In short, we can't fix it. The environments failing use a wrapper that contains

def __getattr__(self, attr): return getattr(self._env, attr)

The problem exists when _env doesn't exist in the wrapper, i.e., when a staticmethod is existed (__setstate__) then this causes an infinite loop to occur of __getattr__("static_method") -> __getattr__("_env") -> __getattr__("_env") -> ad infinitum
The second issue is that dm don't seem to be maintaining the project anymore.

The solution is simple

def __getattr__(self, attr): if "_env" in self.__dict__: return getattr(self._env, attr) else: return super().__getattribute__(attr)

Could you put a PR up by any chance? Even if they don't' end up merging it I feel like we might as well try, you seem to understand this stuff better than me though, I'm not sure I'd be able to explain it well or respond to any questions about it.

google-deepmind/bsuite#49

pseudo-rnd-thoughts · 2023-03-29T22:40:16Z

tests/test_dm_control.py

@@ -103,6 +104,36 @@ def test_seeding(env_id):
    env_2.close()


+@pytest.mark.skip(
+    reason="Fatal Python error: Segmentation fault (with or without EzPickle"


Do you know the source of this? Mujoco is the probable cause which we can probably get solved

Looks like it's to do with pluggy?

/Users/elliottower/anaconda3/envs/shimmy/bin/python /Applications/PyCharm.app/Contents/plugins/python/helpers/pycharm/_jb_pytest_runner.py --target test_dm_control.py::test_pickle Testing started at 1:17 PM ... Launching pytest with arguments test_dm_control.py::test_pickle --no-header --no-summary -q in /Users/elliottower/Documents/GitHub/Shimmy/tests ============================= test session starts ============================== collecting ... collected 85 items test_dm_control.py::test_pickle[dm_control/acrobot-swingup-v0] Fatal Python error: Segmentation fault Current thread 0x000000011149c600 (most recent call first): File "/Users/elliottower/Documents/GitHub/Shimmy/tests/test_dm_control.py", line 114 in test_pickle File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/_pytest/python.py", line 192 in pytest_pyfunc_call File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/pluggy/_callers.py", line 39 in _multicall File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/pluggy/_manager.py", line 80 in _hookexec File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/pluggy/_hooks.py", line 265 in __call__ File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/_pytest/python.py", line 1761 in runtest File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/_pytest/runner.py", line 166 in pytest_runtest_call File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/pluggy/_callers.py", line 39 in _multicall File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/pluggy/_manager.py", line 80 in _hookexec File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/pluggy/_hooks.py", line 265 in __call__ File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/_pytest/runner.py", line 259 in <lambda> File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/_pytest/runner.py", line 338 in from_call File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/_pytest/runner.py", line 258 in call_runtest_hook File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/_pytest/runner.py", line 219 in call_and_report File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/_pytest/runner.py", line 130 in runtestprotocol File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/_pytest/runner.py", line 111 in pytest_runtest_protocol File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/pluggy/_callers.py", line 39 in _multicall File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/pluggy/_manager.py", line 80 in _hookexec File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/pluggy/_hooks.py", line 265 in __call__ File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/_pytest/main.py", line 347 in pytest_runtestloop File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/pluggy/_callers.py", line 39 in _multicall File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/pluggy/_manager.py", line 80 in _hookexec File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/pluggy/_hooks.py", line 265 in __call__ File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/_pytest/main.py", line 322 in _main File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/_pytest/main.py", line 268 in wrap_session File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/_pytest/main.py", line 315 in pytest_cmdline_main File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/pluggy/_callers.py", line 39 in _multicall File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/pluggy/_manager.py", line 80 in _hookexec File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/pluggy/_hooks.py", line 265 in __call__ File "/Users/elliottower/anaconda3/envs/shimmy/lib/python3.9/site-packages/_pytest/config/__init__.py", line 164 in main File "/Applications/PyCharm.app/Contents/plugins/python/helpers/pycharm/_jb_pytest_runner.py", line 51 in <module> Process finished with exit code 139 (interrupted by signal 11: SIGSEGV)

tests/test_dm_control_multi_agent.py

tests/test_dm_lab.py

tests/test_dm_control_multi_agent.py

elliottower · 2023-03-30T17:22:59Z

@pseudo-rnd-thoughts any idea about these? It looks like DM lab seeding doesn't work, and there was some issue with pickling: https://github.com/Farama-Foundation/Shimmy/actions/runs/4567097924/jobs/8060464686

Also I've run the pre-commit hooks locally and it passed previously but now for some reason the CI pre-commit hooks are failing with the meltingpot compatibility file and tests? If you look up a few runs it was passing but I don't think I changed the file. I uninstalled pre-commit and re-installed it and it didn't change anything. Tried reverting to the original changes but it now won't let me commit without it re-formatting things. Not sure if this issue has happened before?

Edit: got it working by commenting out isort on the file, not ideal but the problem persisted even after removing and reinstalling pre-commit hooks.

elliottower · 2023-03-30T17:54:54Z

About the seeding for DM lab, it seems it is possible (link), but our tests here are returning different observations and failing the seed test. I commented out the observations and it still failed with other errors. I think the problem then must be that we are not properly seeding it in the wrapper.

…pytest

…led)

elliottower · 2023-03-30T20:44:03Z

Checked over my own code to ensure there weren't any obvious typos or mistakes, tests are passing now so it should be in a good state to merge at least from what I can see.

pseudo-rnd-thoughts · 2023-03-31T10:00:49Z

shimmy/dm_lab_compatibility.py

@@ -48,6 +48,10 @@ def reset(
        self._env.reset(seed=seed)


I thought we had solved the seeding issue in dm-lab @jjshoots

Not sure but I think this should be a separate PR with the other dm-lab fixes, this is just for adding pickle tests and turns out the pickle tests for dm lab don't seem to work and are blocked so can just update the seed/pickle stuff for dm-lab later

tests/test_meltingpot.py

tests/test_dm_lab.py

pseudo-rnd-thoughts · 2023-03-31T11:09:06Z

tests/test_bsuite.py

+# Without EzPickle:_register_bsuite_envs.<locals>._make_bsuite_env cannot be pickled
+# With EzPickle: maximum recursion limit reached
+FAILING_PICKLE_ENVS = [
+    "bsuite/bandit_noise-v0",


google-deepmind/bsuite#49

shimmy/meltingpot_compatibility.py

pseudo-rnd-thoughts · 2023-03-31T14:05:42Z

Thanks elliot, I agree, just a couple of points about pytest.importorskip then we can merge and make a release

elliottower and others added 9 commits March 27, 2023 18:05

Add multi-agent dm control dockerfile and workflow

7f90eb8

Fix typo in dm control multiagent workflow

b30e99a

Merge remote-tracking branch 'upstream/HEAD' into dm-lab-ci

6a0e2f2

# Conflicts: # .github/workflows/optional-install-test.yml

Add dm-lab dockerfile and workflow

9e3dc4e

Fix typo in dm_lab dockerfile

e8f9d87

Add shimmy[dm-lab] pip installation to match other envs

ae08a50

Add pickling tests for meltingpot, openspiel, bsuite, EzPickle for op…

1bc5a35

…enspiel/bsuite

Add initial pickle test to all third party environments (besides gym)

2e97a55

Merge branch 'main' into pickle-tests

9f0760d

elliottower marked this pull request as ready for review March 29, 2023 20:39

elliottower added 3 commits March 29, 2023 16:50

Update PZ version after 1.22.4 yank

f0afeef

Add importorskip for dm_lab so main tests don't fail

706db5f

Try old import deepmind_lab inside of test_check_env()

dc9df9c

pseudo-rnd-thoughts requested changes Mar 29, 2023

View reviewed changes

pseudo-rnd-thoughts reviewed Mar 30, 2023

View reviewed changes

tests/test_dm_control_multi_agent.py Outdated Show resolved Hide resolved

Add dm-env requirement to dm-lab dockerfile (fix CI error)

5917bd5

elliottower mentioned this pull request Mar 30, 2023

EzPickle support #50

Closed

7 tasks

elliottower added 3 commits March 30, 2023 12:49

Fix typo in multiagent dm control test

55e13a7

Update dm-lab tests to correct action type (from int to dict)

29a0e4f

Fix dm control multiagent init error (recursion limit)

86fc8bc

Add all dm-lab levels to test, comment out obs test (not matching)

efac866

elliottower added 7 commits March 30, 2023 14:03

Attempt to fix dm-lab seeding, fix pickling test typo

45de236

Fix typo in dm lab test

6be6af8

Fix meltingpot isort issues (ignore files, works locally just not in CI)

e6a1c88

Fix dm control to take 10x less time for seed testing (1+hrs currently)

ad564f8

Fix typo in dm lab test

320f498

Make seed warning a print statement so execution doesn't stop during …

4992a4c

…pytest

Skip dm lab tests

084d9b2

elliottower added 7 commits March 30, 2023 15:28

Switch dm lab tests to do lt_chasm (env used in official examples)

ef0f81a

Skip dm lab tests again due to erros

246c8e1

Fix typo in install script

9413b02

Change dm_control_multi_agent test skip reason (weakref can't be pick…

60f7482

…led)

Fix typo in dm control test

0240502

Fix typo in dm control mutliagent test

492a364

Fix typo in dm control multiagent test

aa0bb38

pseudo-rnd-thoughts requested changes Mar 31, 2023

View reviewed changes

elliottower added 3 commits March 31, 2023 10:29

Remove isort ignore and don't run local pre-commit hooks

746a823

Remove repeated noqa ignores for file-wide ignores, remove extra imports

ca39529

Fix pre-commit in meltingpot test

cce032f

pseudo-rnd-thoughts merged commit 5dbaa9a into Farama-Foundation:main Mar 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add pickle tests #53

Add pickle tests #53

elliottower commented Mar 28, 2023 •

edited

Loading

pseudo-rnd-thoughts left a comment

pseudo-rnd-thoughts Mar 29, 2023

elliottower Mar 30, 2023

pseudo-rnd-thoughts Mar 30, 2023 •

edited

Loading

elliottower Mar 30, 2023

pseudo-rnd-thoughts Mar 31, 2023

pseudo-rnd-thoughts Mar 29, 2023

elliottower Mar 30, 2023

elliottower commented Mar 30, 2023 •

edited

Loading

elliottower commented Mar 30, 2023

elliottower commented Mar 30, 2023

pseudo-rnd-thoughts Mar 31, 2023

elliottower Mar 31, 2023

pseudo-rnd-thoughts Mar 31, 2023

pseudo-rnd-thoughts commented Mar 31, 2023

Add pickle tests #53

Add pickle tests #53

Conversation

elliottower commented Mar 28, 2023 • edited Loading

Description

Type of change

Screenshots

Checklist:

pseudo-rnd-thoughts left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pseudo-rnd-thoughts Mar 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elliottower commented Mar 30, 2023 • edited Loading

elliottower commented Mar 30, 2023

elliottower commented Mar 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pseudo-rnd-thoughts commented Mar 31, 2023

elliottower commented Mar 28, 2023 •

edited

Loading

pseudo-rnd-thoughts Mar 30, 2023 •

edited

Loading

elliottower commented Mar 30, 2023 •

edited

Loading