Add is_slippery option for cliffwalking environment

[CloseChoice]

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Supports #161 (issue)

Type of change

Please delete options that are not relevant.

• Documentation only change (no code changed)
• 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.

Checklist:

• I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• 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

[pseudo-rnd-thoughts]

@CloseChoice Could you update the PR with the project main

[CloseChoice]

@CloseChoice Could you update the PR with the project main

just did that.

[pseudo-rnd-thoughts]

Thanks for that @CloseChoice

Is this equivalent to the old version when is_slippery=False?

Would it be possible to include in the reset and step info a probability key that is equal to the probability of the outcome occuring?

[CloseChoice]

Thanks for that @CloseChoice

Is this equivalent to the old version when is_slippery=False?

Would it be possible to include in the reset and step info a probability key that is equal to the probability of the outcome occuring?

Yes, old version and is_slippery=False are identical.

Isn't the probability (or prob) key already implemented on main, see here and here? Or am I missing something here?

[pseudo-rnd-thoughts]

I meant is_slippery=True

Sorry, I didn't realise that prob also existed. Could you add a test that checks that the prob == 1 for is_slippery=False and < 1 for is_slippery=True

Small note: v1 was actually due to the reward threshold being changed (openai/gym#2205 and openai/gym#2315) but kept the comment about is_slippy with a note that this was at v1.0

[CloseChoice]

I meant is_slippery=True

Sorry, I didn't realise that prob also existed. Could you add a test that checks that the prob == 1 for is_slippery=False and < 1 for is_slippery=True

Small note: v1 was actually due to the reward threshold being changed (openai/gym#2205 and openai/gym#2315) but kept the comment about is_slippy with a note that this was at v1.0

I think I tested this already, see here. Do you want this more explicitly, or in some other fashion?

Is there a todo for me regarding the version? Somehow I think I did not register v1 properly but I don't know how to do that, would be great if you could point me in the right direction

EDIT: I saw that you changed the title and also your comment

I meant is_slippery=True

suggests that there is a misunderstanding. The implementation on main of this environment is not slippery, meaning each action can just lead to one state (with transition prob 1). But this PR adds the slippery option. To keep things backwards compatible the default for is_slippery is False which reproduces the current behaviour on main.

[pseudo-rnd-thoughts]

I think I tested this already, see here. Do you want this more explicitly, or in some other fashion?

Sorry, I somehow missed that

suggests that there is a misunderstanding. The implementation on main of this environment is not slippery, meaning each action can just lead to one state (with transition prob 1). But this PR adds the slippery option. To keep things backwards compatible the default for is_slippery is False which reproduces the current behaviour on main.

Your right, I've updated the title, thanks for clarifying

[pseudo-rnd-thoughts]

Looks good to merge