add fastbrili to repo #300

charles-rs · 2023-10-07T19:41:56Z

like brili, but faster. hopefully reasonably correct

Pat-Lafon · 2023-10-09T15:56:36Z

fastbril/README.md

+
+ binary: `make release`
+  the binary will be `./build/fastbrili`
+ doc: you need to have LaTeX installed. run `make doc`


FYI, I tried to run this and got:

Makefile:58: target `src/.h' given more than once in the same rule. Makefile:58: target `src/.h' given more than once in the same rule. Makefile:58: target `src/.h' given more than once in the same rule. Makefile:58: target `src/.h' given more than once in the same rule. Makefile:69: target `doc/.tex' given more than once in the same rule. Makefile:69: target `doc/.tex' given more than once in the same rule. Makefile:69: target `doc/.tex' given more than once in the same rule. Makefile:69: target `doc/.tex' given more than once in the same rule. mkdir -p doc/ ./docgen.sh doc/.tex awk: can't open file config/.cf source line number 0 make: *** [doc/.tex] Error 2

this sounds like find isn't working for you the same way it does for me...

Pat-Lafon · 2023-10-09T16:17:50Z

fastbril/benchmarks/example.toml

+[runs.baseline]
+pipeline = [
+    "bril2json",
+    "\\time -f \"rtime: %R\" brili -p {args}",


It time -f standard? I don't seem to have this on mac.

What is the easy way to hook this into the larger benchmark suite?

-f is standard afaik, but zsh has its own time built-in which is probably what you are seeing on Mac

I'm a hold out that still uses bash on Mac, but fair. Didn't have too much luck with /usr/bin/time but its probably just an issue on my end

I will say all of this is only tested on Linux and the benchmark stuff was mostly for the report, so it might make sense to just remove it here

i told make to use /bin/sh maybe this will fix it?

sampsyo · 2023-10-09T19:12:31Z

Awesome!!! Thanks for getting this PR started!!

One medium-sized thing that would be helpful here would be to reuse the existing benchmark code (benchmarks/ in repo root) instead of having separate copies in here. I'll also look into getting this going on my machine in the mean time… if we get something roughly reproduced, this is probably worth merging.

charles-rs · 2023-10-10T00:27:47Z

Awesome!!! Thanks for getting this PR started!!

One medium-sized thing that would be helpful here would be to reuse the existing benchmark code (benchmarks/ in repo root) instead of having separate copies in here. I'll also look into getting this going on my machine in the mean time… if we get something roughly reproduced, this is probably worth merging.

i don't think there is any reason this should be difficult; we should be able to just use the config files and point them at the other benchmarks; i just need to rebuild all of the 6120 tools which i might not do tonight lol

pass integers as floats and it gets the type from the decltype of the main function

charles-rs · 2023-10-14T18:28:15Z

ok it looks like it is pretty easy to use the existing benchmarks, so i got rid of the redundant one. this also exposed a "bug" where if you pass something like "23" to a program expecting a float it won't work since i was depending on seeing a decimal to parse floats

...instead of shelling out. Ain't the pattern rewriting thingy great?

sampsyo · 2023-11-04T17:24:31Z

I've attempted to fix up a few things that weren't working on my end. In 76949d9, I used built-in GNU Make functionality instead of shelling out to stuff, which fixed a few mysterious problems. In 06d5baf, I bypassed the shell script srcgen.sh, which was just generating filenames and dispatching to awk, and instead had the Makefile supply those filenames directly to the awk script. @charles-rs, any chance you could check that things still work on your end?

One thing I'm still confused about: it looks like this PR has checked-in generated header files. In fact, it has them twice: once under lib and once under src. I think that they should probably be checked in zero times… and just generated during the build. Was that your intent, @charles-rs? If so, which directory are they supposed to go in? We can delete & ignore the generated files, but I want to make sure I'm putting them in the right place. (And TBH I don't understand why there is a lib directory at all, separate from src, so I thought I'd check.)

It also looks like there are a few other generated files that should probably be removed, in eval, but wanted to check before removing those also.

charles-rs · 2023-11-07T02:16:03Z

Thanks so much for getting this working on another system!!

I had to make one more change to the awk shebang invocation to get it to work in my environment, so if you could double check that that would be great

the generated headers were checked in because they weren't originally generated -- good catch

the reason for a separate src and lib directory is that lib contains the (quite minimal) runtime that you need to link against if you are using the compiler feature. The headers aren't even needed here though, so my bad.

re: eval/
this is really a directory with stuff in it that i used for my report. I think it's better to just be rid of it if that's ok with you

Pat-Lafon · 2023-11-09T00:05:45Z

benchmarks/fastbril-turnt.toml

@@ -0,0 +1,4 @@
+# command = "bril2json < {filename} | ../build/fastbrili -p {args}"
+command = "bril2json < {filename} | $FASTBRILI -ni -bo byte-out ; cat byte-out | $FASTBRILI -b {args} ; rm byte-out"


Does this need to be it's own toml file compared to being added to benchmarks/turnt.toml?

no i just forgot you can put multiple things in one toml file 🤦

sampsyo · 2023-12-03T21:12:49Z

It took me a good long while, but I finally got another chance to take a look at this! Thanks again for helping move it forward. 🙏

I made a couple of minor changes:

The Turnt setup now uses the "interpreter mode" instead of explicitly compiling to bitcode first (a little easier, and works with turnt -j).
Remove the eval files.
Remove emission.{h,cbak}, which don't seem to be used. (Everything still seems to compile and run, anyway.)

I wanted to see how we're doing on correctness, so I ran all the benchmarks with turnt -j -e fastbrili benchmarks/**/*.bril. There are a few failures, but actually not that many:

click to show test results for 70 benchmarks

1..70
ok 1 - benchmarks/core/ackermann.bril fastbrili
ok 2 - benchmarks/core/armstrong.bril fastbrili
ok 3 - benchmarks/core/binary-fmt.bril fastbrili
ok 4 - benchmarks/core/birthday.bril fastbrili
ok 5 - benchmarks/core/bitshift.bril fastbrili
ok 6 - benchmarks/core/bitwise-ops.bril fastbrili
ok 7 - benchmarks/core/catalan.bril fastbrili
ok 8 - benchmarks/core/check-primes.bril fastbrili
ok 9 - benchmarks/core/collatz.bril fastbrili
ok 10 - benchmarks/core/digital-root.bril fastbrili
ok 11 - benchmarks/core/dot-product.bril fastbrili
ok 12 - benchmarks/core/euclid.bril fastbrili
ok 13 - benchmarks/core/fact.bril fastbrili
ok 14 - benchmarks/core/factors.bril fastbrili
ok 15 - benchmarks/core/fitsinside.bril fastbrili
ok 16 - benchmarks/core/fizz-buzz.bril fastbrili
ok 17 - benchmarks/core/gcd.bril fastbrili
ok 18 - benchmarks/core/hanoi.bril fastbrili
ok 19 - benchmarks/core/is-decreasing.bril fastbrili
ok 20 - benchmarks/core/lcm.bril fastbrili
ok 21 - benchmarks/core/loopfact.bril fastbrili
ok 22 - benchmarks/core/mod_inv.bril fastbrili
ok 23 - benchmarks/core/orders.bril fastbrili
ok 24 - benchmarks/core/palindrome.bril fastbrili
ok 25 - benchmarks/core/pascals-row.bril fastbrili
ok 26 - benchmarks/core/perfect.bril fastbrili
ok 27 - benchmarks/core/primes-between.bril fastbrili
ok 28 - benchmarks/core/pythagorean_triple.bril fastbrili
ok 29 - benchmarks/core/quadratic.bril fastbrili
ok 30 - benchmarks/core/recfact.bril fastbrili
ok 31 - benchmarks/core/rectangles-area-difference.bril fastbrili
ok 32 - benchmarks/core/relative-primes.bril fastbrili
ok 33 - benchmarks/core/reverse.bril fastbrili
ok 34 - benchmarks/core/sum-bits.bril fastbrili
ok 35 - benchmarks/core/sum-check.bril fastbrili
ok 36 - benchmarks/core/sum-divisors.bril fastbrili
ok 37 - benchmarks/core/sum-sq-diff.bril fastbrili
ok 38 - benchmarks/core/totient.bril fastbrili
ok 39 - benchmarks/core/up-arrow.bril fastbrili
not ok 40 - benchmarks/float/conjugate-gradient.bril fastbrili # differing: benchmarks/float/conjugate-gradient.out
ok 41 - benchmarks/float/cordic.bril fastbrili
not ok 42 - benchmarks/float/euler.bril fastbrili # differing: benchmarks/float/euler.out
ok 43 - benchmarks/float/mandelbrot.bril fastbrili
not ok 44 - benchmarks/float/n_root.bril fastbrili # differing: benchmarks/float/n_root.out
not ok 45 - benchmarks/float/newton.bril fastbrili # differing: benchmarks/float/newton.out
not ok 46 - benchmarks/float/norm.bril fastbrili # differing: benchmarks/float/norm.out
not ok 47 - benchmarks/float/pow.bril fastbrili # differing: benchmarks/float/pow.out
ok 48 - benchmarks/float/ray-sphere-intersection.bril fastbrili
not ok 49 - benchmarks/float/riemann.bril fastbrili # differing: benchmarks/float/riemann.out
not ok 50 - benchmarks/float/sqrt.bril fastbrili # differing: benchmarks/float/sqrt.out
not ok 51 - benchmarks/long/function_call.bril fastbrili # missing: benchmarks/long/function_call.out
ok 52 - benchmarks/mem/adj2csr.bril fastbrili
ok 53 - benchmarks/mem/adler32.bril fastbrili
ok 54 - benchmarks/mem/binary-search.bril fastbrili
ok 55 - benchmarks/mem/bubblesort.bril fastbrili
ok 56 - benchmarks/mem/csrmv.bril fastbrili
ok 57 - benchmarks/mem/eight-queens.bril fastbrili
ok 58 - benchmarks/mem/fib.bril fastbrili
ok 59 - benchmarks/mem/major-elm.bril fastbrili
ok 60 - benchmarks/mem/mat-mul.bril fastbrili
ok 61 - benchmarks/mem/max-subarray.bril fastbrili
ok 62 - benchmarks/mem/primitive-root.bril fastbrili
ok 63 - benchmarks/mem/quickselect.bril fastbrili
ok 64 - benchmarks/mem/quicksort-hoare.bril fastbrili
ok 65 - benchmarks/mem/quicksort.bril fastbrili
ok 66 - benchmarks/mem/sieve.bril fastbrili
ok 67 - benchmarks/mem/two-sum.bril fastbrili
ok 68 - benchmarks/mem/vsmul.bril fastbrili
not ok 69 - benchmarks/mixed/cholesky.bril fastbrili # differing: benchmarks/mixed/cholesky.out
not ok 70 - benchmarks/mixed/mat-inv.bril fastbrili # differing: benchmarks/mixed/mat-inv.out

The only failure mode I noticed while spot-checking is about float printing. For example:

$ turnt -e fastbrili --diff benchmarks/float/conjugate-gradient.bril
1..1
--- benchmarks/float/conjugate-gradient.out	2023-09-08 13:36:07
+++ /var/folders/dj/jj7bq9q570908ggkt0vcfvch0000gn/T/tmprv_jbljs	2023-12-03 16:07:20
@@ -1,3 +1,3 @@
-5.00000000000000000
-3.00000000000000000
-2.33333333333333348
+5
+3
+2.3333333333333335
not ok 1 - benchmarks/float/conjugate-gradient.bril fastbrili # differing: benchmarks/float/conjugate-gradient.out

Unfortunately, this line doesn't quite cut it:

bril/fastbril/lib/lib.c

Line 18 in 96aca05

printf("%.17g", d);

But we may be able to fix it by borrowing the silliness I worked out for Brilift:

bril/brilift/rt.c

Lines 20 to 32 in daaff28

    
           void _bril_print_float(double f) { 
        
               if (isnan(f)) { 
        
                   printf("NaN"); 
        
               } else if (isinf(f)) { 
        
                   if (f < 0) { 
        
                       printf("-Infinity"); 
        
                   } else { 
        
                       printf("Infinity"); 
        
                   } 
        
               } else { 
        
                   printf("%.17lf", f); 
        
               } 
        
           }

In fact, someday maybe we should share the same runtime source code between the two, since essentially the identical functionality is required in both implementations.

Anyway, I think I would like to merge this now, if @charles-rs approves of the above changes! We can address the two things I have in mind in separate PRs:

Conformant float printing
Write a page for the public-facing docs

…and then maybe we can enable testing in CI.

charles-rs · 2023-12-29T16:34:39Z

thanks so much for doing this! sorry it fell off the radar for a bit. Good call on the emission files, those were for a separate (incomplete) feature. This looks good to me to merge!

sampsyo · 2024-01-02T15:31:46Z

Awesome!! Thanks again for the really cool interpreter!!!!

add fastbril to repo

28e57b5

Pat-Lafon reviewed Oct 9, 2023

View reviewed changes

Charles Sherk added 2 commits October 9, 2023 20:19

check in documentation; update makefile and gitignore

801db70

udpate readme

eab0f26

yeet local benchmarks, fix bug with arg parsing so that you can now

3eb4127

pass integers as floats and it gets the type from the decltype of the main function

sampsyo added 4 commits November 4, 2023 13:08

Fix Makefile by using make functions

76949d9

...instead of shelling out. Ain't the pattern rewriting thingy great?

Remove & ignore doc product

bbba781

Fix Makefile's use of awk converter

06d5baf

Remove srcgen.sh

df048f1

remove and ignore generated files

77971ed

Pat-Lafon reviewed Nov 9, 2023

View reviewed changes

Charles Sherk and others added 8 commits November 25, 2023 17:34

move fastbrili turnt into main file

47cb6f9

move fastbrili turnt into main file

a088259

Merge branch 'main' of github.com:charles-rs/bril

9a04b18

Merge branch 'main' of github.com:charles-rs/bril into charles-rs/main

fea021b

Use "interpreter mode" to test fastbrili

006375e

Remove evaluation data

37680cc

Remove a stray lockfile

bcbf046

Remove emission sources (which seem unused)

96aca05

sampsyo merged commit 188cd38 into sampsyo:main Jan 2, 2024
3 checks passed

sampsyo mentioned this pull request Jan 6, 2024

fastbrili: Conformant float printing #309

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add fastbrili to repo #300

add fastbrili to repo #300

charles-rs commented Oct 7, 2023

Pat-Lafon Oct 9, 2023

charles-rs Oct 19, 2023

Pat-Lafon Oct 9, 2023

Pat-Lafon Oct 9, 2023

charles-rs Oct 9, 2023

Pat-Lafon Oct 9, 2023

charles-rs Oct 9, 2023

charles-rs Oct 10, 2023

sampsyo commented Oct 9, 2023

charles-rs commented Oct 10, 2023

charles-rs commented Oct 14, 2023

sampsyo commented Nov 4, 2023

charles-rs commented Nov 7, 2023

Pat-Lafon Nov 9, 2023

charles-rs Nov 25, 2023

sampsyo commented Dec 3, 2023 •

edited

Loading

charles-rs commented Dec 29, 2023

sampsyo commented Jan 2, 2024

		@@ -0,0 +1,4 @@
		# command = "bril2json < {filename} \| ../build/fastbrili -p {args}"
		command = "bril2json < {filename} \| $FASTBRILI -ni -bo byte-out ; cat byte-out \| $FASTBRILI -b {args} ; rm byte-out"

add fastbrili to repo #300

add fastbrili to repo #300

Conversation

charles-rs commented Oct 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sampsyo commented Oct 9, 2023

charles-rs commented Oct 10, 2023

charles-rs commented Oct 14, 2023

sampsyo commented Nov 4, 2023

charles-rs commented Nov 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sampsyo commented Dec 3, 2023 • edited Loading

charles-rs commented Dec 29, 2023

sampsyo commented Jan 2, 2024

sampsyo commented Dec 3, 2023 •

edited

Loading