-
Notifications
You must be signed in to change notification settings - Fork 21
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
Add pickle tests #53
Merged
pseudo-rnd-thoughts
merged 34 commits into
Farama-Foundation:main
from
elliottower:pickle-tests
Mar 31, 2023
Merged
Add pickle tests #53
Changes from 1 commit
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
7f90eb8
Add multi-agent dm control dockerfile and workflow
elliottower b30e99a
Fix typo in dm control multiagent workflow
elliottower 6a0e2f2
Merge remote-tracking branch 'upstream/HEAD' into dm-lab-ci
elliottower 9e3dc4e
Add dm-lab dockerfile and workflow
elliottower e8f9d87
Fix typo in dm_lab dockerfile
elliottower ae08a50
Add shimmy[dm-lab] pip installation to match other envs
elliottower 1bc5a35
Add pickling tests for meltingpot, openspiel, bsuite, EzPickle for op…
elliottower 2e97a55
Add initial pickle test to all third party environments (besides gym)
elliottower 9f0760d
Merge branch 'main' into pickle-tests
elliottower f0afeef
Update PZ version after 1.22.4 yank
elliottower 706db5f
Add importorskip for dm_lab so main tests don't fail
elliottower dc9df9c
Try old import deepmind_lab inside of test_check_env()
elliottower 5917bd5
Add dm-env requirement to dm-lab dockerfile (fix CI error)
elliottower 55e13a7
Fix typo in multiagent dm control test
elliottower 29a0e4f
Update dm-lab tests to correct action type (from int to dict)
elliottower 86fc8bc
Fix dm control multiagent init error (recursion limit)
elliottower efac866
Add all dm-lab levels to test, comment out obs test (not matching)
elliottower 45de236
Attempt to fix dm-lab seeding, fix pickling test typo
elliottower 6be6af8
Fix typo in dm lab test
elliottower e6a1c88
Fix meltingpot isort issues (ignore files, works locally just not in CI)
elliottower ad564f8
Fix dm control to take 10x less time for seed testing (1+hrs currently)
elliottower 320f498
Fix typo in dm lab test
elliottower 4992a4c
Make seed warning a print statement so execution doesn't stop during …
elliottower 084d9b2
Skip dm lab tests
elliottower ef0f81a
Switch dm lab tests to do lt_chasm (env used in official examples)
elliottower 246c8e1
Skip dm lab tests again due to erros
elliottower 9413b02
Fix typo in install script
elliottower 60f7482
Change dm_control_multi_agent test skip reason (weakref can't be pick…
elliottower 0240502
Fix typo in dm control test
elliottower 492a364
Fix typo in dm control mutliagent test
elliottower aa0bb38
Fix typo in dm control multiagent test
elliottower 746a823
Remove isort ignore and don't run local pre-commit hooks
elliottower ca39529
Remove repeated noqa ignores for file-wide ignores, remove extra imports
elliottower cce032f
Fix pre-commit in meltingpot test
elliottower File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
"""Tests the functionality of the DmControlCompatibility Wrapper on dm_control envs.""" | ||
import pickle | ||
import warnings | ||
from typing import Callable | ||
|
||
|
@@ -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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you know the source of this? Mujoco is the probable cause which we can probably get solved There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like it's to do with pluggy?
|
||
) | ||
@pytest.mark.parametrize("env_id", DM_CONTROL_ENV_IDS) | ||
def test_pickle(env_id): | ||
"""Test that dm-control seeding works.""" | ||
env_1 = gym.make(env_id) | ||
env_2 = pickle.loads(pickle.dumps(env_1)) | ||
|
||
if "lqr" in env_id or (env_1.spec is not None and env_1.spec.nondeterministic): | ||
# LQR fails this test currently. | ||
return | ||
|
||
obs_1, info_1 = env_1.reset(seed=42) | ||
obs_2, info_2 = env_2.reset(seed=42) | ||
assert data_equivalence(obs_1, obs_2) | ||
assert data_equivalence(info_1, info_2) | ||
for _ in range(100): | ||
actions = env_1.action_space.sample() | ||
obs_1, reward_1, term_1, trunc_1, info_1 = env_1.step(actions) | ||
obs_2, reward_2, term_2, trunc_2, info_2 = env_2.step(actions) | ||
assert data_equivalence(obs_1, obs_2) | ||
assert reward_1 == reward_2 | ||
assert term_1 == term_2 and trunc_1 == trunc_2 | ||
assert data_equivalence(info_1, info_2) | ||
|
||
env_1.close() | ||
env_2.close() | ||
|
||
|
||
@pytest.mark.parametrize("camera_id", [0, 1]) | ||
def test_rendering_camera_id(camera_id): | ||
"""Test that dm-control rendering works.""" | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 functionThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In short, we can't fix it. The environments failing use a wrapper that contains
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 infinitumThe second issue is that dm don't seem to be maintaining the project anymore.
The solution is simple
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
google-deepmind/bsuite#49