-
Notifications
You must be signed in to change notification settings - Fork 223
Added HillClimbing First Diagram for Chapter 4 #79
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
Conversation
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.
This is really cool! I played it a few times and it made me think about search strategy in a way that I wouldn't have if I only read about it.
|
||
constructor(hill) { | ||
this.hill = hill; | ||
this.currentState = Math.floor(Math.random() * (this.hill.getStates().length + 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.
What does the +1 do here?
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.
Just when I started to type on why this +1 was used, I realized it was a mistake! I used +1 because of the Math.floor(). I thought if +1 was not used, the random number will be in range 0 to n-1 instead of 0 to n. I mistakenly thought the latter is what we required. I will fix it.
} | ||
|
||
constructor(states) { | ||
this.states = (typeof states != 'undefined') ? states : this.generateRandomHill(); |
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 you can use states !== undefined
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.
Yeah, this is much simpler
this.hillClimber = new HillClimber(this.hill); | ||
this.hillDiagram = new HillDiagram(this.svg, this.hill, this.h, this.w); | ||
this.hillClimberDiagram = new HillClimberDiagram(this.h, this.w, this.svg, this.hill, this.hillDiagram, this.hillClimber); | ||
this.bindClicks(); |
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.
Minor suggestion: the second constructor takes svg, hill, h, w, and the third constructor takes h, w, svg, hill; it'd look a little nicer if they used a consistent order
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.
Will fix this
this.currentDecision = this.DECISIONS.STAY; | ||
class Hill { | ||
|
||
getRandomValues() { |
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.
This is clever! I tried running it several times and each time it gave me a new set of nice hills.
|
||
generateRandomHill() { | ||
this.getRandomValues(); | ||
let states = []; |
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.
There's some weird indentation here. It looks like it is a tab size issue in your editor. If you can, set the indentation to 2 spaces, but the physical tab size to 8 spaces to match github and other applications. Or set it to always use spaces.
Updated in accordance with the suggested changes. |
Text Added, Moves increased to 25 and a small png image added to represent the robot (instead of a red block). This PR can be merged now. |
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.
Where is the robot from? Is there a license file that goes with it? By default the things we check in are covered by our copyright and license, and we don't want to claim copyright over the robot unless you made it. We can put any libraries/images we didn't make into a subdirectory ("third-party/") and include the copyright/license information.
BTW http://game-icons.net/tags/robot.html and http://www.flaticon.com/search?word=robot have SVG icons, which would allow us to recolor them from d3 (I think). That would allow the robot to be one color when exploring and another color to indicate failure/success. (And they're creative commons attribution licensed). Another place you can find icons is by using emoji; that's how this simulation tool gets its graphics :-) |
Nice ! I got that image from https://commons.wikimedia.org/wiki/File:Noun_project_1248.svg |
It looks like it comes from here: https://thenounproject.com/term/robot/1248/ . I don't think there's a standard way to mention license information but the simplest would be to put a
The readme file will be for people browsing the source. We also need something for people viewing the page. At the bottom of the page you could put |
Done ! |
This adds the first Diagram for Chapter 4. Progress in #58
It is a small game where the user can use a limited amount of clicks to reveal the hill and try to find the global maxima.
Hill generation uses Random Values at specific intervals and cosine interpolation between them.
The number of moves to be given to the user is not figured yet.
Texts will be added next.
@redblobgames Take a look, please.