Feature/add benchmarks [in progre 8000 ss] #385
Conversation
|
Nice! The first thing that would be great here would be a short paragraph for each of these about what it is testing, what is actually happening. Next thing is at least a text description in the namespace that describes how to run the benchmarks. Is there some manual timing report generated that we can copy in to the namespace, commented out? |
| @@ -10,4 +10,5 @@ | |||
| org.clojure/core.match {:mvn/version "1.0.0"}, | |||
| cljsjs/complex {:mvn/version "2.0.11-0"}, | |||
| borkdude/sci {:mvn/version "0.2.5"}, | |||
| cljsjs/bigfraction {:mvn/version "4.1.1-0"}}} | |||
| cljsjs/bigfraction {:mvn/version "4.1.1-0"} | |||
| criterium/criterium {:mvn/version "0.4.4"}}} | |||
There was a problem hiding this comment.
Adding this here will make criterium a dependency of the whole project, which we don't want. Can you add a separate build target that adds this to dependencies? There should be some model out there, though I don't have my head around deps.edn style yet. But imagine that this is the same problem as a testing target, where you don't want to ship the testing framework out to folks that are consuming the library.
There was a problem hiding this comment.
If I understand this guide correctly, we should be able to do something like this in deps.edn,
;; deps.edn
{:paths ["src"]
:deps ...whichever deps we have right now...
:aliases {:test {:extra-paths ["test"]
:extra-deps ...tests-specific deps...}
:benchmark {:extra-paths ["benchmark"]
:extra-deps {criterium/criterium {:mvn/version "0.4.4"}}
}}}right?
benchmarks/benchmakrs.clj
Outdated
|
|
||
| (defn forward-pass [ws-layers] | ||
| (fn [input] | ||
| (vec (flatten (map #(reducers/reduce sum-layer % ws-layers) input))))) |
There was a problem hiding this comment.
I need to stare at this for a bit before I can comment well, but flatten is almost never needed - flatten flattens arbitrary layers of nesting, and it is usually cleaner to flatten just the layers that you want, using perhaps mapcat or apply concat.
Sorry about my lazy code formatting, I'm trying to get used to an ergo keyboard to save my pinkies and code is REALLY tough to type!
There was a problem hiding this comment.
Tried to remove these, still not sure I am doing this optimally, any feedback would be much appreciated :-)
afa6dd7 to
c33ef42
Compare
An initial attempt at addressing https://github.com/sicmutils/placeholder/issues/86 .
Currently dumps everything in a single file in a
benchmarksfolder. Eventually, we might want to consider doing something like Stan does where benchmarks are run on every PR (via Jenkins) and generate a report on the PR itself; this makes it easy to see the performance implications of the suggested change, which is nice.This is WIP, would be happy to get feedback on what would be a good next step, here.