matsud224 / wamcompiler

Prolog implementation based on Warren's abstract machine
The Unlicense
41 stars 6 forks source link

Great work, but ... #1

Closed FemtoEmacs closed 4 years ago

FemtoEmacs commented 4 years ago

Wamcompiler is a very impressive piece of software. As far as I know, it is the first decent implementation of Prolog in Common Lisp. Many people tried and failed in this endeavor. For instance, Paul Graham, Peter Norvig, Mauro Jacob and Patrice Boizumalt were not able to implement Prolog in Lisp. You did it and deserve the credit and praise for a great work. However, there is a small bug in your implementation. I hope that you fix it as soon as possible, so that my students can start using your Prolog. Let us see an example of the problem:

~/wrkLisp/src/wamcompiler master •
› rlwrap ros run
* (load "wamcompiler.lisp")
* (repl)
> gn(N, 42) :- N =< 4, write(small), nl.
> gn(N, 43) :- N > 4, write(large), nl.
> ?- gn(2, Res).
small
Res = 42
?;

no.
> ?- gn(8, Res).
large
Res = 43

yes.

%% So far, so good. That is exactly what I was expecting.
%% Now, let us add a cut to the first clause:

> fn(N, 42) :- N =< 4, ! , write(small), nl.
> fn(N, 43) :- N > 4, write(large), nl.
> ?- fn(2, G).

It does not work. I believe that the problem lies in the cut. I have a few other suggestion for your consideration, after fixing the bug, of course.

Suggestion 1: Add prototypes in order to avoid warnings. Here are the prototypes that you should add at the beginning of the wamcompiler.lisp file:

(declaim (ftype (function (t) t) set-to-trail))
(declaim (ftype (function (t) t) send-query))
(declaim (ftype (function (t) t) prolog-expr->string))
(declaim (ftype (function (t &optional t) t) compile-clause))
(declaim (ftype (function (t) t) divide-head-body))
(declaim (ftype (function (t t) t) optimize-wamcode))
(declaim (ftype (function (t) t) skip-comment))
(declaim (ftype (function (t) t) skip-whitespace))
(declaim (ftype (function (t) t) read-non-alphanum-atom))
(declaim (ftype (function (t) t) read-num))
(declaim (ftype (function (t) t) read-quoted-atom))
(declaim (ftype (function (t) t) read-alphanum-atom))
(declaim (optimize (speed 3) (space 0) (debug 0) (safety 0)))

Suggestion 2: Add general numeric operations. For instance, you can replace read-int with read-num, as shown below:

(defun get-token (s)
  (skip-whitespace s)
  (let ((c (read-char s)))
    (cond ((null c) nil)
      ((eq c #\%) (skip-comment s) (get-token s))
      ((eq c #\)) '(rparen))
      ((eq c #\() '(lparen))
      ((eq c #\]) '(rbracket))
      ((eq c #\[) '(lbracket))
      ((eq c #\|) '(vertical-bar))
      ((eq c #\,) (cons 'atom '|,|))
      ((eq c #\;) (cons 'atom '|;|))
      ((eq c #\!) (cons 'atom '|!|))
      ((eq c #\') (read-quoted-atom s))
      ((digit-char-p c) (unread-char c s) (cons 'atom (read-num s) ))
      ((member c *non-alphanum-chars*)
       (unread-char c s) (read-non-alphanum-atom s))
      (t (unread-char c s) (read-alphanum-atom s)))))

;;; ... stuff

(defun read-num (s)
  (let* ( (acc)
      (point t))
    (dostream (c s)
          (unless (or (digit-char-p c)
              (and point (equal #\. c)))
        (unread-char c s) (return nil))
          (setq acc (cons c acc))
          (when (equal c #\.) (setq point nil)))
    (cond ( (equal (car acc) #\.) (unread-char (car acc) s)
          (parse-integer (concatenate 'string (reverse (cdr acc))) ))
      (point (parse-integer (concatenate 'string (reverse acc)) ))
      (t (parse-string-to-float
          (concatenate 'string (reverse acc))) ))))

Suggestion 3: Add a few optimizations, which can boost speed by 50% at least. For instance, just by adding a declaration of type to a few definitions, I was able to boost speed by 40 percent. Here is an example:

(defun heap (addr)
  (declare (optimize (speed 3) (space 0) (debug 0) (safety 0))
       (type fixnum addr))
  (aref *heap-area* (1- (- (/ addr 2)))))

My experience shows that one can obtain huge improvements by using arrays of numbers, instead of general Lisp symbolic expressions. I wonder if you could replace at least one of the prolog stacks with a numerical stack. I did this in the Boizumalt Prolog, and it bacame three times faster.

matsud224 commented 4 years ago

I'm glad you used wamcompiler and thank you for finding some bugs! I worked on developing wamcompiler about 3 years ago. So, I don't remember well about it and can't take time for now :crying_cat_face:. But I'll welcome your pull-request!

matsud224 commented 4 years ago

I translated some Japanese comments in wamcompiler.lisp and README to English.