Skip to content
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

101 decoupled action spaces #102

Merged
merged 15 commits into from
Dec 27, 2023
Merged

101 decoupled action spaces #102

merged 15 commits into from
Dec 27, 2023

Conversation

AhmadAmine998
Copy link
Collaborator

No description provided.

@AhmadAmine998 AhmadAmine998 added the enhancement New feature or request label Dec 19, 2023
@AhmadAmine998 AhmadAmine998 linked an issue Dec 19, 2023 that may be closed by this pull request
@AhmadAmine998 AhmadAmine998 self-assigned this Dec 22, 2023

self.steering_low, self.steering_high = params["s_min"], params["s_max"]
self.velocity_low, self.velocity_high = params["v_min"], params["v_max"]
self._type = "speed"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo? also should add steering so it's specified

class SteeringSpeedAction(SteerAction):
def __init__(self, params: Dict) -> None:
super().__init__()
self._type = "speed"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add steering so it's specified


@staticmethod
def from_string(action: str):
if action == "angle":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, should specifiy steering_angle and steering_speed, just to be less confusing

@hzheng40
Copy link
Member

@AhmadAmine998 I pulled in the updated tests and it's failing right now. You'll probably have to update the tests to use the decoupled action spaces.
Also, should maybe consider handling when only one action type is set, (throw a warning and setting the other to default).

@AhmadAmine998
Copy link
Collaborator Author

@AhmadAmine998 I pulled in the updated tests and it's failing right now. You'll probably have to update the tests to use the decoupled action spaces. Also, should maybe consider handling when only one action type is set, (throw a warning and setting the other to default).

I updated the initialization of the action space to be compatible with specifying only one control input as a string. The old tests passed and I now updated the tests to use the decoupled action specification. This way it is compatible with legacy code while still having updated tests

Copy link
Member

@hzheng40 hzheng40 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@hzheng40 hzheng40 merged commit 0f3b630 into v1.0.0 Dec 27, 2023
4 checks passed
@hzheng40 hzheng40 mentioned this pull request Dec 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

Decoupled Action Spaces
2 participants