-
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
Add continuous maze example. #216
base: main
Are you sure you want to change the base?
Conversation
7a10ee0
to
05a7747
Compare
src/main/scala/symsim/examples/concrete/continuousmaze/ContinuousMaze.scala
Show resolved
Hide resolved
src/main/scala/symsim/examples/concrete/continuousmaze/ContinuousMaze.scala
Show resolved
Hide resolved
src/main/scala/symsim/examples/concrete/continuousmaze/ContinuousMaze.scala
Outdated
Show resolved
Hide resolved
val TimeHorizon: Int = 2000 | ||
|
||
def isFinal (s: MazeState): Boolean = | ||
s == MazeState (MAZE_LENGTH, MAZE_WIDTH) || s == MazeState (MAZE_LENGTH, MAZE_WIDTH - 1) |
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.
Should isFinal
be defined on MazeState
or on MazeObservableState
?
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.
According to the agent class, it should be maze state.
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.
Then, I think the code is incorrect either here or there. If isFinal
is called on observable state, then the set of the final states should be an entire square, not just two points. If isFinal
is called by symsim on observable state, then they should require the observable state type, not the system state type? Either side is confused. It does not make sense to check an observable state condition on a continuous state ...
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.
Then, I think the code is incorrect either here or there. If
isFinal
is called on observable state, then the set of the final states should be an entire square, not just two points. IfisFinal
is called by symsim on observable state, then they should require the observable state type, not the system state type? Either side is confused. It does not make sense to check an observable state condition on a continuous state ...
Maybe I wasn't clear, I meant it is defined on MazeState
. If it was defined on MazeObservableState
, you were right.
src/main/scala/symsim/examples/concrete/continuousmaze/ContinuousMaze.scala
Outdated
Show resolved
Hide resolved
case MazeState (_, _) => -1.0 | ||
|
||
|
||
def distort (a: MazeAction): Randomized[MazeAction] = a match |
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 the continuous, version of the problem it would be more natural to change the distance moved, not the direction. But anyhow, how are you going to treat randomization in the symbolic executor?
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 have an idea that we do not need to do anything about randomization. Let's talk about it in detail in the meeting. Just as a short answer, when there is randomization, it means that instead of a1, the behavior of a2 must be considered. Since we are analyzing the program for all actions, then we should not be worried about randomized actions.
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.
But here a good randomization would not select a different action, but would result in adding random noise (for instance Gaussian) to the distance travelled.
src/main/scala/symsim/examples/concrete/continuousmaze/ContinuousMaze.scala
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,19 @@ | |||
package symsim | |||
package examples.concrete.continuousmaze |
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 the package name should be continuousMaze
not continuousmaze
.
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 tried to keep it similar to the other packages. e.g. simplemaze, simplebandit, ...
Should I rename it?
src/test/scala/symsim/examples/concrete/continuousmaze/SarsaExperiments.scala
Outdated
Show resolved
Hide resolved
I have left my comments, but I am confused why tests for |
@wasowski |
BTW. Andreas tells me that this example is basically the same thing that is known as the random walk example. |
Is it? |
@wasowski
Please consider reviewing this example and let me know whether it is what we want.