Skip to content
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

Implementation of Natural Numbers #138

Merged
merged 11 commits into from
Aug 29, 2023
Merged

Implementation of Natural Numbers #138

merged 11 commits into from
Aug 29, 2023

Conversation

agureev
Copy link
Collaborator

@agureev agureev commented Jul 10, 2023

  1. Introduces SeqN category of finite natural number sequences

  2. Introduces a new pipeline compiling STLC terms through SeqN instead of BITC

  3. Implements for each n an object of natural numbers of n-bit length alongside appropriate floored operations alongside new API. Propagated the implementation to STLC and SeqN.

  4. Implements is-less-than and is-equal-to predicates on natural numbers in Geb, propagates the change to STLC and SeqN.

  5. Upgrades the pipeline now supporting compilation of STLC+Natural numbers+Natural Number Predicates to Vamp-IR

@agureev agureev force-pushed the artem/nat-implementation branch from 96f799c to fa5e375 Compare August 4, 2023 13:30
@agureev agureev marked this pull request as ready for review August 4, 2023 13:30
@agureev agureev requested a review from mariari August 4, 2023 13:30
Copy link
Member

@mariari mariari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah see all my comments, they are fairly through.

In the future take the generic work into their own topics so I can merge them faster and be done more properly.

When changes are in big prs like this I get less stringent and works worse with a bigger team size.

Also the docs won't build this isn't your fault (I don't think besides warnings you produced), but by other issues I've posted in #140

Comment on lines 312 to 316
:body (list (make-bind :names (list wire-ai)
:value (make-application :func :fresh
:arguments (list (make-infix :op :\|
:lhs one
:rhs wire-a))))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's start with a general formatting note

     :body (list (make-bind :names (list wire-ai)
                            :value (make-application
                                    :func :fresh
                                    :arguments (list (make-infix :op :\|
                                                                 :lhs one
                                                                 :rhs wire-a))))

For code like this try to make it look nice and around 80, here is an example of what you can do around code here in general that gets big. Notice we don't compromise the formatting too much to do this, so it read fine.

(make-application :func :minus_range
:arguments (list n x1 x2)))

(defparameter *isZero*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note symbols are case insensitive, so this will be the same as *ISZERO* or *iszero* but I don't mind it

Comment on lines 226 to 227
:inputs (list :n
:a)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor formatting nit, you can 1 line the list here fine

Comment on lines 258 to 260
:inputs (list :n
:x1
:x2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same fam

:name :take_ind
:inputs (list :take
:|(h:t)|)
:body (list (make-wire :var :|h:(take t)|))))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- :body (list (make-wire :var :|h:(take t)|))))
+  :body (list (make-infix :lhs (make-wire :var :h)
+                           :rhs (make-application :func :take
+                                                  :arguments (list (make-wire :var :t)))
+                           :op :|:|))

Notice that we can use application here, and it is much more proper against any syntax changes that vampir might get.

I want all of these changes. Infix places parens for you already properly

VAMPIR> (make-infix :lhs (make-wire :var :h) 
                    :op :+
                    :rhs
                    (make-infix :lhs (make-wire :var :h)
                                :rhs (make-application :func :take :arguments (list (make-wire :var :t)))
                                :op :|:|))
h + (h : (take t))

if you need an extra set for whatever reason, then you should add that as a construct

Comment on lines 236 to 328
((plus ltm rtm) (let ((tyltm (ttype tterm)))
;; tyltm derived type problems
;; Generally can simplify all
;; bit operation code
(comp (mcase (terminal (prod (maybe tyltm)
so1))
(commutes-left (comp (mcase (terminal (prod tyltm
so1))
(nat-add tyltm))
(distribute (ttype ltm)
so1
(ttype ltm)))))
(comp (distribute (maybe tyltm)
so1
(ttype ltm))
(geb:pair (rec context ltm)
(rec context rtm))))))
((minus ltm rtm) (let ((tyltm (ttype ltm)))
(comp (mcase (terminal (prod (maybe tyltm)
so1))
(commutes-left (comp (mcase (terminal (prod tyltm
so1))
(nat-sub tyltm))
(distribute (ttype ltm)
so1
(ttype ltm)))))
(comp (distribute (maybe tyltm)
so1
(ttype ltm))
(geb:pair (rec context ltm)
(rec context rtm))))))
((times ltm rtm) (let ((tyltm (ttype ltm)))
(comp (mcase (terminal (prod (maybe tyltm)
so1))
(commutes-left (comp (mcase (terminal (prod tyltm
so1))
(nat-mult tyltm))
(distribute (ttype ltm)
so1
(ttype ltm)))))
(comp (distribute (maybe tyltm)
so1
(ttype ltm))
(geb:pair (rec context ltm)
(rec context rtm))))))
((divide ltm rtm) (let ((tyltm (ttype ltm)))
(comp (mcase (terminal (prod (maybe tyltm)
so1))
(commutes-left (comp (mcase (terminal (prod tyltm
so1))
(nat-div tyltm))
(distribute (ttype ltm)
so1
(ttype ltm)))))
(comp (distribute (maybe tyltm)
so1
(ttype ltm))
(geb:pair (rec context ltm)
(rec context rtm))))))
((lamb-eq ltm rtm) (let ((tyltm (ttype ltm)))
(comp (mcase (terminal (prod (maybe tyltm)
so1))
(commutes-left (comp (mcase (terminal (prod tyltm
so1))
(geb.extension.spec:nat-eq tyltm))
(distribute (ttype ltm)
so1
(ttype ltm)))))
(comp (distribute (maybe tyltm)
so1
(ttype ltm))
(geb:pair (rec context ltm)
(rec context rtm))))))
((lamb-lt ltm rtm) (let ((tyltm (ttype ltm)))
(comp (mcase (terminal (prod (maybe tyltm)
so1))
(commutes-left (comp (mcase (terminal (prod tyltm
so1))
(geb.extension.spec:nat-lt tyltm))
(distribute (ttype ltm)
so1
(ttype ltm)))))
(comp (distribute (maybe tyltm)
so1
(ttype ltm))
(geb:pair (rec context ltm)
(rec context rtm))))))
((bit-choice bitv) (comp (comp (->right so1 (ttype tterm))
(nat-const (ttype tterm) (reduce
#'(lambda (a b)
(+ (ash a 1) b))
bitv)))
(terminal (stlc-ctx-maybe context))))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see lambda is still horrible as ever.

Let us attept to clean it up.

Please fix the formatting by newlining after (bit-choice bitv), so the code it maps to is on the next line, given just how big they are.

Comment on lines 164 to 167
(mapcar (lambda (x) 0)
(mcadr morphism))))
(inj-length-right (append (mapcar (lambda (x) 0)
(mcar morphism))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sbcl will yell at you for unused x. just use the make-list I used in my previous comments for these. would read much better. Or in this case use the pad-with-zeros I mentioned before.

Maybe make the API of pad-with-zeros take a keyword for prepend or postpend, doing the more useful one by default.

src/geb/geb.lisp Outdated
Comment on lines 542 to 577
(defmethod well-defp-cat ((morph <substmorph>))
(etypecase-of substmorph morph
(comp (let ((mcar (mcar morph))
(mcadr (mcadr morph)))
(if (and (well-defp-cat mcar)
(well-defp-cat mcadr)
(obj-equalp (dom mcar)
(codom mcadr)))
t
(error "(Co)Domains do not match for ~A" morph))))
(case (let ((mcar (mcar morph))
(mcadr (mcadr morph)))
(if (and (well-defp-cat mcar)
(well-defp-cat mcadr)
(obj-equalp (codom mcar)
(codom mcadr)))
t
(error "Casing ill-defined for ~A" morph))))
(pair (let ((mcar (mcar morph))
(mcdr (mcdr morph)))
(if (and (well-defp-cat mcar)
(well-defp-cat mcdr)
(obj-equalp (dom mcar)
(dom mcdr)))
t
(error "Pairing ill-defined for ~A" morph))))
((or substobj
init
terminal
project-left
project-right
inject-left
inject-right
distribute)
t)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah just newline the code after the case.

(defmethod well-defp-cat ((morph <substmorph>))
  (etypecase-of substmorph morph
    (comp
     (let ((mcar (mcar morph))
           (mcadr (mcadr morph)))
       (if (and (well-defp-cat mcar)
                (well-defp-cat mcadr)
                (obj-equalp (dom mcar)
                            (codom mcadr)))
           t
           (error "(Co)Domains do not match for ~A" morph))))
    (case
        (let ((mcar (mcar morph))
              (mcadr (mcadr morph)))
          (if (and (well-defp-cat mcar)
                   (well-defp-cat mcadr)
                   (obj-equalp (codom mcar)
                               (codom mcadr)))
              t
              (error "Casing ill-defined for ~A" morph))))
    (pair
     (let ((mcar (mcar morph))
           (mcdr (mcdr morph)))
       (if (and (well-defp-cat mcar)
                (well-defp-cat mcdr)
                (obj-equalp (dom mcar)
                            (dom mcdr)))
           t
           (error "Pairing ill-defined for ~A" morph))))
    ((or substobj
         init
         terminal
         project-left
         project-right
         inject-left
         inject-right
         distribute)
     t)))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future, make such generic functions be a separate topic. Thus this should be apart of some other topic, and merged into your topic.

For this PR I'll let you keep it here, but note for the future, will make your work diverge less in a real setting of X engineers hacking on a project and if I was an active dev I could merge this patch into my own code

Comment on lines 198 to 241

(defmethod well-defp-cat ((morph <seqn>))
(etypecase-of seqn morph
(composition (let* ((mcar (mcar morph))
(mcadr (mcadr morph))
(dom (dom mcar))
(cod (cod mcadr)))
(if (and (well-defp-cat mcar)
(well-defp-cat mcadr)
(obj-equalp dom
cod))
t
(error "Co(Domains) do not match for ~A with domain
of MCAR ~A1 and codomain of MCADR ~A2."
morph cod dom))))
(parallel-seq (if (and (well-defp-cat (mcar morph))
(well-defp-cat (mcadr morph)))
t
(error "Not well-defined parallel composition ~A" morph)))
(branch-seq (let* ((mcar (mcar morph))
(mcadr (mcadr morph))
(dom1 (dom mcar))
(dom2 (dom mcadr))
(cod1 (cod mcar))
(cod2 (cod mcadr)))
(if (and (well-defp-cat mcar)
(well-defp-cat mcadr)
(obj-equalp dom1 dom2)
(obj-equalp cod1 cod2))
t
(error "Not a well-defined branching ~A.
~A1 has dom ~a1 and cod ~a2.
~A2 has dom ~a3 and cod ~a4"
morph mcar dom1 cod1 mcadr dom2 cod2))))
(shift-front (if (>= (length (mcar morph))
(mcadr morph))
t
(error "Wrong shift-length for ~A" morph)))
((or id
fork-seq
drop-nil
drop-width
remove-right
remove-left
inj-length-left
inj-length-right
inj-size
zero-bit
one-bit
seqn-add
seqn-multiply
seqn-divide
seqn-subtract
seqn-nat
seqn-concat
seqn-decompose
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same formatting comment as I mentioned earlier

test/seqn.lisp Outdated
Comment on lines 12 to 22
(define-test seqn-de-compose
:parent geb-seqn
(is obj-equalp (list 1 0) (gapply (geb.seqn.spec:seqn-decompose 3) (list 4)))
(is obj-equalp (list 4) (gapply (geb.seqn.spec:seqn-concat 1 2) (list 1 0)))
(is obj-equalp (list 4) (gapply (geb.seqn.spec:composition (geb.seqn.spec:seqn-concat 1 2)
(geb.seqn.spec:seqn-decompose 3))
(list 4)))
(is obj-equalp (list 1 0) (gapply (geb.seqn.spec:composition (geb.seqn.spec:seqn-decompose 3)
(geb.seqn.spec:seqn-concat 1 2))
(list 1 0))))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alias your seqn so this is just seqn: and remove the seqn words on your functions they only make the API ugly

Copy link
Member

@mariari mariari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah see all my comments, they are fairly through.

In the future take the generic work into their own topics so I can merge them faster and be done more properly.

When changes are in big prs like this I get less stringent and works worse with a bigger team size.

Also the docs won't build this isn't your fault (I don't think besides warnings you produced), but by other issues I've posted in #140

@agureev agureev force-pushed the artem/nat-implementation branch from fa5e375 to 1a45f5d Compare August 28, 2023 13:32
Adds two new constrictors to the VampIR spec, allowing for making of
the empty bracket used for induction on lists and of the curly
brackets for function specification.
Adds a new primitive operation of conditional division to the VampIR
spec. The primitive stands for the VampIR operation which is capable
of dividing by 0, producing 0.
@agureev agureev force-pushed the artem/nat-implementation branch 3 times, most recently from 4a2508e to 3a0f998 Compare August 29, 2023 16:39
agureev and others added 5 commits August 30, 2023 01:50
Upgrade the Vamp-IR package to include basic functions of the library
such as checking whether something is zero, checking whether a number
is negative etc. Alongside that, arithmetic operations with appropriate range
checks are introduced in order to be used in the upgraded pipeline
which will allow for the compilation of natural number functions and
predicates of fixed bitwidth.
Upgrades the standard library needed to compile JuvixCore functions
containing natural numbers functions and predicates.
Introduces a new category SeqN with appropriate API alongside with
the compilation of it to appropriate Vamp-IR code. SeqN can be
thought of as a category of finite sequences of natural numbers
with morphisms from (x1,...,xn) to (y1,...,ym) being Vamp-IR
functions taking in n entries of sizes x1 to xn and spitting out
m outputs of sizes y1 to ym.
Makes the so-eval function into a generic function in order to be
compatible with further Geb extensions.
@mariari mariari force-pushed the artem/nat-implementation branch from 3a0f998 to 3b52547 Compare August 29, 2023 19:43
Upgrades the pipeline to now support compiling to Vamp-IR through
SeqN. That includes adding natural number support for STLC, adding
natural numbers in a Geb extension and subsequently upgrading
appropriate compilation steps with a new pass from Geb to SeqN.
1) Upgrades gapply for Geb to allow interpetation of natural numbers
extension morphisms whose inputs will be lists of natural
numbers.

2) Adds gapply for SeqN, computing appropriate natural number inputs.
Updates test files to include interpretations of compiled STLC code in
SeqN and other individual tests for interpreters in Geb and SeqN.
@mariari mariari force-pushed the artem/nat-implementation branch from 3b52547 to b843889 Compare August 29, 2023 20:09
@mariari mariari merged commit 6c45e7e into main Aug 29, 2023
@mariari mariari deleted the artem/nat-implementation branch August 29, 2023 20:20
@agureev agureev mentioned this pull request Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants