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

Support for custom observations (both observation function and observation space) #133

Merged
merged 10 commits into from
Feb 24, 2023

Conversation

firemankoxd
Copy link
Contributor

Hello Lucas,
as we discussed in PR-126, I separated the observations to another file and solved the previous issue - where the user might specify only observation function without specifying observation space.

User can now specify the observations by creating class which will be inherited from base ObservationFunction class and adding it as a observation_class parameter when creating SumoEnvironment object. If the observation class is not defined, DefaultObservationFunction is used instead, so this change shouldn't affect previous implementations.

One thing I am not completely sure of is the discrete_observation_space attribute of the TrafficSignal class. I didn't find any usage of this attribute so I am not sure if it should be also considered in observations. Or maybe I can just delete that attribute in another commit? 😄 Please, let me know.

@LucasAlegre LucasAlegre self-requested a review February 24, 2023 11:47
@LucasAlegre LucasAlegre added the enhancement New feature or request label Feb 24, 2023
@LucasAlegre
Copy link
Owner

Hi @firemankoxd , thanks a lot for these contributions! I will try to review them as soon as possible :)

@LucasAlegre
Copy link
Owner

The PR looks great! Thanks again! :D

The discrete_observation_space is indeed useless at this point. I will remove it next.

@LucasAlegre LucasAlegre merged commit bd8ef1d into LucasAlegre:master Feb 24, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants