Min/max/if optimizations based on the insights from rival-analyze #87
Conversation
…min/max/if optimizations
…to min-max-optimizations
eval/run.rkt
Outdated
| #:unless (and (not first-iter?) repeat)) | ||
| [hint (if vhint | ||
| (in-vector vhint) | ||
| (in-producer (const #t)))] |
There was a problem hiding this comment.
If no vhint is provided then execute all the instructions and treat local hint as #t
…erged flag for rival-analyze
eval/main.rkt
Outdated
| (define-values (hint hint-converged?) (make-hint machine)) | ||
| (list (ival (or bad? stuck?) (not good?)) hint hint-converged?)) | ||
|
|
||
| (module+ test |
There was a problem hiding this comment.
The tests represent a random testing of the hints implementation.
It samples random hyperrects in a global box of [-100, 100] and points inside these hyperrects.
The hyperrects are analyzed using rival-analyze and obtained hint is executed for points.
The tests verify that execution with hint and without hint produces the same results.
Additionally, the tests check the percentage of instruction executions being skipped by hint.
There was a problem hiding this comment.
Can you move the tests to a separate file? Otherwise, sure.
There was a problem hiding this comment.
I did not review the tests closely.
eval/adjust.rkt
Outdated
| (define bool-reg (vector-ref vregs bool-idx)) | ||
| (match* ((ival-lo bool-reg) (ival-hi bool-reg) (ival-err? bool-reg)) | ||
| [(#t #t #f) ; assert and its children should not be reexecuted if it is true already | ||
| (vhint-set! bool-idx (or #f (vhint-ref bool-idx))) |
There was a problem hiding this comment.
This is a no-op, right? (or #f x) is just x.
There was a problem hiding this comment.
My thinking was like this:
since we are using batches, we may have duplicates and this why I made or operation.
The thing is that make-hint function first will create a mask for all the instructions,
and mark #t nodes that are roots (they always need to be executed to write some result in vreg vector).
Then it starts a traverse in the reverse order.
This line of code catches the following case:
Imagine that we have a list of expressions as:
(list '(< (cos x) 0) '(assert (< (cos x) 0)))
The algorithm first will mark roots < and assert as #t - should be executed.
And then, the algorithm will start to analyze instruction by instruction.
It will look at assert ... and figure out that assert, for example, is always true - which means that (< (cos x) 0) basically should not be executed.
And here a mistake can happen, (< (cos x) 0) actually should be executed because it is another root and some result should be written into vregs, if we mark (< (cos x) 0) as #f - there will be no output in vregs.
Therefore, the algorithm before marking anything as #f checks whether it previously was marked as #t and if not - it is safe to mark it as #f, otherwise, the instruction should be executed
There was a problem hiding this comment.
Yeah I get all that. My proposal is delete this line of code entirely. If no one else wants this line of code, its hint is already set to #f (that's the default). If someone else wants it, good, we don't disturb it.
There was a problem hiding this comment.
Oh, right right, here this line of code is useless
There was a problem hiding this comment.
I guess the same operation can be removed everywhere
eval/main.rkt
Outdated
| (define-values (hint hint-converged?) (make-hint machine)) | ||
| (list (ival (or bad? stuck?) (not good?)) hint hint-converged?)) | ||
|
|
||
| (module+ test |
There was a problem hiding this comment.
Can you move the tests to a separate file? Otherwise, sure.
eval/main.rkt
Outdated
| (define-values (hint hint-converged?) (make-hint machine)) | ||
| (list (ival (or bad? stuck?) (not good?)) hint hint-converged?)) | ||
|
|
||
| (module+ test |
There was a problem hiding this comment.
I did not review the tests closely.
Co-authored-by: Pavel Panchekha <me@pavpanchekha.com>
This PR introduces an optimization for
fmin,fmaxandifoperations.Particularly speaking, sometimes the computational path of an expression is the same for a range of inputs and having an insight would speed up some evaluations.
A simple example is:
(if (> (exp x) 3) (cos x) (sin x)).Given that
exp,cosandsinare all expensive functions to compute, that would be nice to have some insights aboutthe condition result of
(> (exp x) 3)to avoid additional computations.As known,
rival-analyzecan analyze an expression for a range of input variables and conclude about error status for the whole range.It has been observed, that
rival-analyzemay also give a hint on computational path of the expression.For example, after analyzing
(if (> (exp x) 3) (cos x) (sin x))forxlarger than 2, the function would provide a hintthat for any
xthat is larger than 2, only(cos x)should be executed.Therefore, no
(> (exp x) 3), no(sin x)should be executed at all.This optimization has been implemented and it is supposed to speed up some particular benchmarks with a large number of
fmax,fminandifoperations.