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

Benchmark sanitization and minor additions #1207

Closed
wants to merge 2 commits into from
Closed

Conversation

Izaakwltn
Copy link
Collaborator

Addressing issue #1204, this sanitizes the directory while we figure out how to replace trivial-benchmarks.

  • Changes the benchmarks library to package-per-file
  • Removes trivial-benchmarks as a dependency

This also adds two benchmarks from the Benchmarks Game, with hopeful additions coming soon.

I think next steps (for future PR(s)) should include a simple Coalton benchmarking framework, ideally without external dependencies, and incorporation of benchmarks into CI.

Copy link
Member

@stylewarning stylewarning left a comment

Choose a reason for hiding this comment

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

this PR is still a draft and needs a lot of preparation

Due to dependency issues, currently all benchmarks are packaged and run separately.

They are intended to be incorporated into a cohesive benchmark suite using a coalton benchmarking framework at a later date.
Copy link
Member

Choose a reason for hiding this comment

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

if this doesn't get deleted, then capitalize Coalton

Due to dependency issues, currently all benchmarks are packaged and run separately.
Copy link
Member

Choose a reason for hiding this comment

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

This sounds like airing dirty laundry. I'd reword this to just be direct. "This system contains a variety of individual benchmarks. This system does not presently have a single, unified benchmark suite."

Copy link
Member

Choose a reason for hiding this comment

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

Maybe state that by convention, functions ending in -main are intended to be a single reasonable and repeatable benchmarking function requiring no arguments.

Other conventions can be stated as well. Maybe

benchmark-*

should be any kind of function intended to be a benchmark, and

main-benchmark

should always be the name of the main benchmark of a package?

-> (Complex :a)
-> Integer))
(define (escapes? limit fc div? z0)
"Returns the number of iterations of the function F "
Copy link
Member

Choose a reason for hiding this comment

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

document -1

@@ -0,0 +1,179 @@
(defpackage #:coalton/benchmarks/benchmarks-game/mandelbrot
Copy link
Member

Choose a reason for hiding this comment

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

we should write a comment on what this code is benchmarking. i don't mean documenting what mandelbrot is, but rather "this code benchmarks highly polymorphic complex numerics, and high order functions. the code is written in a relatively idiomatic functional style." or so


(define (too-simple-main n)
(time (fn ()
(let sum = (cell:new 0.0d0))
Copy link
Member

Choose a reason for hiding this comment

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

lift the fn out into its own function


(define (too-simple-main2 n)
(time (fn ()
(let ((declare run (Double-Float -> Double-Float -> Double-Float -> Double-Float))
Copy link
Member

Choose a reason for hiding this comment

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

same

(let x = (into x))
(* x (* pi ee))))))

;;(cl:in-package #:coalton-benchmarks)
Copy link
Member

Choose a reason for hiding this comment

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

dead code


;;(cl:in-package #:coalton-benchmarks)

#+ig
Copy link
Member

Choose a reason for hiding this comment

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

figure out what to do with these


;;(cl:in-package #:coalton-benchmarks)

#+ig (define-benchmark recursive-fib ()
Copy link
Member

Choose a reason for hiding this comment

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

figure out what to do here.

@Izaakwltn Izaakwltn marked this pull request as draft August 13, 2024 07:02
@Izaakwltn
Copy link
Collaborator Author

Whoops, I thought I marked it as a draft

@Izaakwltn
Copy link
Collaborator Author

Closing this as it's superseded by #1286

The additional benchmarks defined in this PR will be added to a new PR after #1286 merges.

@Izaakwltn Izaakwltn closed this Nov 7, 2024
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