-
Notifications
You must be signed in to change notification settings - Fork 0
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
Updating master #26
Merged
Updating master #26
Conversation
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
Make grid configurable from text file
Add continuous state space; update tests and docs
+ Now the agent configuration allows to specify the initial position.
+ Added new module: core.action The purpose of this module is to merge in one single place the definition and the effect of the various action spaces. We can now delegate the effect of an action to the action itself. The agent doesn't need to distinguish them. This is necessary in order to define new action spaces in users of the repo, without modifying this any further. - Transition not complete.
+ Enum is not an integer now: I need the step method, so not an int. + Two default actions added as abstract methods: nop and bip. + Removed all code that this module is intended to substitute. - Needs testing now.
+ Updated version because this branch introduces incompatible changes + Small fixes and type checks
+ wrappers.observations contains three common observation spaces that can be extract from sapientino dict. Other subclassese many define different ones. - Tests to be added.
+ The configuration now accepts a map as a string. This is more general than files, and easier to use in tests. + Now working tests with tested observation spaces. + Small fixes.
+ Tested all new features. + Now observations are always numpy arrays because that's what what the gym interface requires.
+ Tested action and observation space with 45 degrees.
Observation space
+ Data is always cancelled from one robot to the next. No easy way to store persistent info on a robot. Why this need? To implement an action it could be useful to keep track of the past, but actions are Enumerations here, and configurations are frozen.
+ The step, reset, render API changed. - To be tested; I'm unsure about the render functionality.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Proposed changes
Hello @marcofavorito .
I want to update one plot for my phd thesis and I need to retrain one piece from the AAAI paper. While I was checking the old code I noticed this develop branch hanging around in this package. We never merged it to master! My bad probably.. Actually I opened a PR some time ago, but now I wanted to open a new one because that one was old now.
I remember some positive things about this branch. The main ones were:
wrappers.observations.UseFeatures
wrapper. Simple enough.core.actions.Command
subclasses when initializing each agent. This is passed to the "core.configuration.SapientinoAgentConfiguration". Thanks to this, the sapientino environment is both a grid-world domain and a continuous environment.Unfortunately, time has passed, there are many things I don't remember well, but I think there's value in this branch and I would like to merge it anyway.
If you agree, here's how I would proceed. I update the dependencies in a separate branch. We merge to develop, then we merge to master. Updating the dependencies first simplifies things a lot. Now Gym is Gynmasium, and testing the old package at this deprecated state would be pointless and time consuming.
What do you think?
Roberto
Tests don't pass yet because I can't execute at this stage. The PR is a draft for now
Types of changes
What types of changes does your code introduce?
Put an
x
in the boxes that apply