From f4cd6351a387ec3174862288cf5a15aa244578a2 Mon Sep 17 00:00:00 2001 From: Eli Barzilay Date: Sat, 20 Dec 2014 12:33:05 -0500 Subject: [PATCH 1/6] Make it possible to read the code without having my eyes bleed. --- rkt.rkt | 93 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 45 insertions(+), 48 deletions(-) diff --git a/rkt.rkt b/rkt.rkt index d2d6be4..a00b386 100644 --- a/rkt.rkt +++ b/rkt.rkt @@ -1,54 +1,51 @@ - #lang typed/racket +#lang typed/racket - (struct: route ([dest : Integer] [cost : Integer]) #:transparent) +(struct: route ([dest : Integer] [cost : Integer]) #:transparent) - (struct: node ([neighbours : (Listof route)]) #:transparent) +(struct: node ([neighbours : (Listof route)]) #:transparent) - (: str->int (String -> Integer)) - (define (str->int str) - (define n (string->number str)) - (if n (numerator (inexact->exact (real-part n))) 0)) +(: str->int : String -> Integer) +(define (str->int str) + (define n (string->number str)) + (if n (numerator (inexact->exact (real-part n))) 0)) - (: read-places (-> (Vectorof node))) - (define (read-places) - (define lines - (file->lines "agraph")) - (define num-lines (str->int (car lines))) - (define nodes (build-vector num-lines (lambda (n) (node `())))) - (let loop ([i : Integer 0]) - (define nums (string-split (list-ref (cdr lines) i))) - (define len (length nums)) - (when (and (> len 2) (> (length lines) (+ i 2))) - (let ([node-id (str->int (list-ref nums 0))] - [neighbour (str->int (list-ref nums 1))] - [cost (str->int (list-ref nums 2))]) - (define new-node (node - (append (node-neighbours (vector-ref nodes node-id)) - (list (route neighbour cost))))) - (vector-set! nodes node-id new-node) - (loop (+ i 1))))) - nodes) +(: read-places : -> (Vectorof node)) +(define (read-places) + (define lines (file->lines "agraph")) + (define num-lines (str->int (car lines))) + (define nodes (build-vector num-lines (lambda (n) (node `())))) + (let loop ([i : Integer 0]) + (define nums (string-split (list-ref (cdr lines) i))) + (define len (length nums)) + (when (and (> len 2) (> (length lines) (+ i 2))) + (define node-id (str->int (list-ref nums 0))) + (define neighbour (str->int (list-ref nums 1))) + (define cost (str->int (list-ref nums 2))) + (define new-node + (node (append (node-neighbours (vector-ref nodes node-id)) + (list (route neighbour cost))))) + (vector-set! nodes node-id new-node) + (loop (+ i 1)))) + nodes) - (: get-longest-path ((Vectorof node) Integer (Vectorof Boolean) -> Integer)) - (define (get-longest-path nodes node-id visited) - (vector-set! visited node-id #t) - (define sum - (foldr - (lambda ([neighbour : route] [max : Integer]) - (if (not (vector-ref visited (route-dest neighbour))) - (let ([dist (+ (route-cost neighbour) (get-longest-path nodes (route-dest neighbour) visited))]) - (if (> dist max) - dist - max)) - max)) - 0 - (node-neighbours (vector-ref nodes node-id)))) - (vector-set! visited node-id #f) - sum) +(: get-longest-path : (Vectorof node) Integer (Vectorof Boolean) -> Integer) +(define (get-longest-path nodes node-id visited) + (vector-set! visited node-id #t) + (define (step [neighbour : route] [max : Integer]) + (if (vector-ref visited (route-dest neighbour)) max + (let ([dist (+ (route-cost neighbour) + (get-longest-path nodes (route-dest neighbour) + visited))]) + (if (> dist max) dist max)))) + (define sum + (foldr step 0 (node-neighbours (vector-ref nodes node-id)))) + (vector-set! visited node-id #f) + sum) - (define nodes (read-places)) - (define visited : (Vectorof Boolean) (build-vector (vector-length nodes) (lambda (n) #f))) - (define start (current-inexact-milliseconds)) - (define len (get-longest-path nodes 0 visited)) - (define duration (- (current-inexact-milliseconds) start)) - (printf "~a LANGUAGE Racket ~a\n" len (inexact->exact (floor duration))) \ No newline at end of file +(define nodes (read-places)) +(define visited : (Vectorof Boolean) + (build-vector (vector-length nodes) (lambda (n) #f))) +(define start (current-inexact-milliseconds)) +(define len (get-longest-path nodes 0 visited)) +(define duration (- (current-inexact-milliseconds) start)) +(printf "~a LANGUAGE Racket ~a\n" len (inexact->exact (floor duration))) From 2d3e1fe89ecdf7aedaba3172e13e9967c0a5b991 Mon Sep 17 00:00:00 2001 From: Eli Barzilay Date: Sat, 20 Dec 2014 12:35:56 -0500 Subject: [PATCH 2/6] Solve the mess around `str->int`. The thing that tends to trip people is that the `integer?` predicate is true for both exact integers (like 123) *and* inexact ones (like 123.0), while Typed Racket's `Integer` corresponds only with the exact ones. So usually the easy way out of this is to use `exact-integer?` instead. As a side note, "The `if n` is like a form of pattern matching" -- that's not really true. There's no pattern matching happenning. Instead, once you use a predicate for a specific type with something in an `if`, Typed Racket knows that in the then-branch, something has that type. In the `if n` case, that means that it knows that it cannot be `#f`, and if outside the `if` it was `(U #f Whatever)` then in the then branch it is left as `Whatever`. This is called "occurrence typing". --- rkt.rkt | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/rkt.rkt b/rkt.rkt index a00b386..6997389 100644 --- a/rkt.rkt +++ b/rkt.rkt @@ -6,8 +6,7 @@ (: str->int : String -> Integer) (define (str->int str) - (define n (string->number str)) - (if n (numerator (inexact->exact (real-part n))) 0)) + (assert (string->number str) exact-integer?)) (: read-places : -> (Vectorof node)) (define (read-places) @@ -15,12 +14,12 @@ (define num-lines (str->int (car lines))) (define nodes (build-vector num-lines (lambda (n) (node `())))) (let loop ([i : Integer 0]) - (define nums (string-split (list-ref (cdr lines) i))) + (define nums (map str->int (string-split (list-ref (cdr lines) i)))) (define len (length nums)) (when (and (> len 2) (> (length lines) (+ i 2))) - (define node-id (str->int (list-ref nums 0))) - (define neighbour (str->int (list-ref nums 1))) - (define cost (str->int (list-ref nums 2))) + (define node-id (list-ref nums 0)) + (define neighbour (list-ref nums 1)) + (define cost (list-ref nums 2)) (define new-node (node (append (node-neighbours (vector-ref nodes node-id)) (list (route neighbour cost))))) From 8f941482f5e5f5be74d4e92d2e17d7ef791b706c Mon Sep 17 00:00:00 2001 From: Eli Barzilay Date: Sat, 20 Dec 2014 12:37:51 -0500 Subject: [PATCH 3/6] Improve reading code. * Use a plain `for` loop over the input instead of an indirect counter * Use `(cons x y)` to add things on the left side instead of `(append l (list x))` (if you really need to preserve the order, then it's better to `reverse` the lists when you're done reading) * Using plain `'` quote is generally preferred when you don't need the extra features of quasi-quotes * `str->int` is used only in `read-places`, so put it there --- rkt.rkt | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/rkt.rkt b/rkt.rkt index 6997389..d3e123e 100644 --- a/rkt.rkt +++ b/rkt.rkt @@ -4,27 +4,22 @@ (struct: node ([neighbours : (Listof route)]) #:transparent) -(: str->int : String -> Integer) -(define (str->int str) - (assert (string->number str) exact-integer?)) - (: read-places : -> (Vectorof node)) (define (read-places) + (define (str->int [str : String]) + (assert (string->number str) exact-integer?)) (define lines (file->lines "agraph")) (define num-lines (str->int (car lines))) - (define nodes (build-vector num-lines (lambda (n) (node `())))) - (let loop ([i : Integer 0]) - (define nums (map str->int (string-split (list-ref (cdr lines) i)))) - (define len (length nums)) - (when (and (> len 2) (> (length lines) (+ i 2))) - (define node-id (list-ref nums 0)) - (define neighbour (list-ref nums 1)) - (define cost (list-ref nums 2)) - (define new-node - (node (append (node-neighbours (vector-ref nodes node-id)) - (list (route neighbour cost))))) - (vector-set! nodes node-id new-node) - (loop (+ i 1)))) + (define nodes (build-vector num-lines (lambda (n) (node '())))) + (for ([3nums (in-list (rest lines))]) + (define nums (map str->int (string-split 3nums))) + (define node-id (first nums)) + (define neighbour (second nums)) + (define cost (third nums)) + (define new-node + (node (cons (route neighbour cost) + (node-neighbours (vector-ref nodes node-id))))) + (vector-set! nodes node-id new-node)) nodes) (: get-longest-path : (Vectorof node) Integer (Vectorof Boolean) -> Integer) From caba702dd979df0d749fa57e94c8830d6d88eaef Mon Sep 17 00:00:00 2001 From: Eli Barzilay Date: Sat, 20 Dec 2014 12:40:52 -0500 Subject: [PATCH 4/6] Functional: pass around a list of visited node numbers instead of a global vector. Makes it three times slower, but much more readable code. --- rkt.rkt | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/rkt.rkt b/rkt.rkt index d3e123e..9abfa00 100644 --- a/rkt.rkt +++ b/rkt.rkt @@ -22,24 +22,17 @@ (vector-set! nodes node-id new-node)) nodes) -(: get-longest-path : (Vectorof node) Integer (Vectorof Boolean) -> Integer) +(: get-longest-path : (Vectorof node) Integer (Listof Integer) -> Integer) (define (get-longest-path nodes node-id visited) - (vector-set! visited node-id #t) - (define (step [neighbour : route] [max : Integer]) - (if (vector-ref visited (route-dest neighbour)) max - (let ([dist (+ (route-cost neighbour) - (get-longest-path nodes (route-dest neighbour) - visited))]) - (if (> dist max) dist max)))) - (define sum - (foldr step 0 (node-neighbours (vector-ref nodes node-id)))) - (vector-set! visited node-id #f) - sum) + (define (step [neighbour : route] [best : Integer]) + (if (memq (route-dest neighbour) visited) best + (max best (+ (route-cost neighbour) + (get-longest-path nodes (route-dest neighbour) + (cons node-id visited)))))) + (foldl step 0 (node-neighbours (vector-ref nodes node-id)))) (define nodes (read-places)) -(define visited : (Vectorof Boolean) - (build-vector (vector-length nodes) (lambda (n) #f))) (define start (current-inexact-milliseconds)) -(define len (get-longest-path nodes 0 visited)) +(define len (get-longest-path nodes 0 '())) (define duration (- (current-inexact-milliseconds) start)) (printf "~a LANGUAGE Racket ~a\n" len (inexact->exact (floor duration))) From 67fca0571e84c7bca37ccfd1bd829dcfc237e6ae Mon Sep 17 00:00:00 2001 From: Eli Barzilay Date: Sat, 20 Dec 2014 12:41:33 -0500 Subject: [PATCH 5/6] Functional: use objects instead of numbers pointing to objects. (Functional in the proper sense, not in the we-don't-mutate sense.) --- rkt.rkt | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/rkt.rkt b/rkt.rkt index 9abfa00..f8377c1 100644 --- a/rkt.rkt +++ b/rkt.rkt @@ -1,10 +1,10 @@ #lang typed/racket -(struct: route ([dest : Integer] [cost : Integer]) #:transparent) +(struct: route ([dest : node] [cost : Integer]) #:transparent) -(struct: node ([neighbours : (Listof route)]) #:transparent) +(struct: node ([neighbours : (Listof route)]) #:transparent #:mutable) -(: read-places : -> (Vectorof node)) +(: read-places : -> node) (define (read-places) (define (str->int [str : String]) (assert (string->number str) exact-integer?)) @@ -13,26 +13,24 @@ (define nodes (build-vector num-lines (lambda (n) (node '())))) (for ([3nums (in-list (rest lines))]) (define nums (map str->int (string-split 3nums))) - (define node-id (first nums)) - (define neighbour (second nums)) + (define node (vector-ref nodes (first nums))) + (define neighbour (vector-ref nodes (second nums))) (define cost (third nums)) - (define new-node - (node (cons (route neighbour cost) - (node-neighbours (vector-ref nodes node-id))))) - (vector-set! nodes node-id new-node)) - nodes) + (set-node-neighbours! node (cons (route neighbour cost) + (node-neighbours node)))) + (vector-ref nodes 0)) -(: get-longest-path : (Vectorof node) Integer (Listof Integer) -> Integer) -(define (get-longest-path nodes node-id visited) +(: get-longest-path : node (Listof node) -> Integer) +(define (get-longest-path node visited) (define (step [neighbour : route] [best : Integer]) (if (memq (route-dest neighbour) visited) best (max best (+ (route-cost neighbour) - (get-longest-path nodes (route-dest neighbour) - (cons node-id visited)))))) - (foldl step 0 (node-neighbours (vector-ref nodes node-id)))) + (get-longest-path (route-dest neighbour) + (cons node visited)))))) + (foldl step 0 (node-neighbours node))) -(define nodes (read-places)) +(define root (read-places)) (define start (current-inexact-milliseconds)) -(define len (get-longest-path nodes 0 '())) +(define len (get-longest-path root '())) (define duration (- (current-inexact-milliseconds) start)) (printf "~a LANGUAGE Racket ~a\n" len (inexact->exact (floor duration))) From fbdad358dcdb756c4881563d66c1368af67f37a1 Mon Sep 17 00:00:00 2001 From: Eli Barzilay Date: Sat, 20 Dec 2014 12:42:17 -0500 Subject: [PATCH 6/6] Back to mutation for visited flags, only keep it in the objects. Makes it roughly as fast as the global-vector version. --- rkt.rkt | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/rkt.rkt b/rkt.rkt index f8377c1..c07bfb5 100644 --- a/rkt.rkt +++ b/rkt.rkt @@ -2,7 +2,8 @@ (struct: route ([dest : node] [cost : Integer]) #:transparent) -(struct: node ([neighbours : (Listof route)]) #:transparent #:mutable) +(struct: node ([neighbours : (Listof route)] [visited? : Boolean]) + #:transparent #:mutable) (: read-places : -> node) (define (read-places) @@ -10,7 +11,7 @@ (assert (string->number str) exact-integer?)) (define lines (file->lines "agraph")) (define num-lines (str->int (car lines))) - (define nodes (build-vector num-lines (lambda (n) (node '())))) + (define nodes (build-vector num-lines (lambda (n) (node '() #f)))) (for ([3nums (in-list (rest lines))]) (define nums (map str->int (string-split 3nums))) (define node (vector-ref nodes (first nums))) @@ -20,17 +21,18 @@ (node-neighbours node)))) (vector-ref nodes 0)) -(: get-longest-path : node (Listof node) -> Integer) -(define (get-longest-path node visited) +(: get-longest-path : node -> Integer) +(define (get-longest-path node) (define (step [neighbour : route] [best : Integer]) - (if (memq (route-dest neighbour) visited) best + (if (node-visited? (route-dest neighbour)) best (max best (+ (route-cost neighbour) - (get-longest-path (route-dest neighbour) - (cons node visited)))))) - (foldl step 0 (node-neighbours node))) + (get-longest-path (route-dest neighbour)))))) + (set-node-visited?! node #t) + (begin0 (foldl step 0 (node-neighbours node)) + (set-node-visited?! node #f))) (define root (read-places)) (define start (current-inexact-milliseconds)) -(define len (get-longest-path root '())) +(define len (get-longest-path root)) (define duration (- (current-inexact-milliseconds) start)) (printf "~a LANGUAGE Racket ~a\n" len (inexact->exact (floor duration)))