Closed avibryant closed 4 years ago
Seems like the lack of inlining is making tests timeout. Will keep this PR unmerged until the follow-ups have resolved that.
Merging #444 into 0.3-dev will decrease coverage by
3.65%
. The diff coverage is79.41%
.
@@ Coverage Diff @@
## 0.3-dev #444 +/- ##
===========================================
- Coverage 54.67% 51.02% -3.66%
===========================================
Files 73 74 +1
Lines 2758 2834 +76
Branches 178 180 +2
===========================================
- Hits 1508 1446 -62
- Misses 1250 1388 +138
Impacted Files | Coverage Δ | |
---|---|---|
...main/scala/com/stripe/rainier/compute/Bounds.scala | 89.79% <ø> (-0.21%) |
:arrow_down: |
...a/com/stripe/rainier/bench/stan/GLMMPoisson2.scala | 0% <0%> (ø) |
:arrow_up: |
...ain/scala/com/stripe/rainier/compute/RealViz.scala | 0% <0%> (ø) |
:arrow_up: |
...in/scala/com/stripe/rainier/compute/Gradient.scala | 93.1% <100%> (-0.12%) |
:arrow_down: |
...cala/com/stripe/rainier/compute/Coefficients.scala | 88.88% <100%> (-6.28%) |
:arrow_down: |
.../com/stripe/rainier/ir/OutputMethodGenerator.scala | 100% <100%> (ø) |
:arrow_up: |
...n/scala/com/stripe/rainier/compute/Evaluator.scala | 63.63% <100%> (+2.76%) |
:arrow_up: |
...ain/scala/com/stripe/rainier/ir/DataFunction.scala | 100% <100%> (ø) |
:arrow_up: |
...in/scala/com/stripe/rainier/compute/Compiler.scala | 100% <100%> (ø) |
:arrow_up: |
.../scala/com/stripe/rainier/compute/LogLineOps.scala | 87.5% <100%> (-12.5%) |
:arrow_down: |
... and 20 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update e24a092...7a0cfd3. Read the comment docs.
cc @andrew-stripe for visibility
This makes a pair of individually conceptually small (if invasive) changes, but that combine to be quite significant:
Coefficients
, used in bothLine
andLogLine
, now usesConstant
nodes to hold the constant coefficient values, rather than directly using numeric valuesColumn
is switched from being treated asNonConstant
to being treated asConstant
The overall effect here is that vector data (from using
Fn
) gets propagated through the graph rather than staying as leaf nodes, providing similar pre-computation benefits in theFn
case as in the direct case.We aren't using
Decimal
, so two tests needed to be disabled. TheDecimal
logic will be brought back and folded intoConstant
.