Page MenuHomePhabricator

Separate the "configuration" and "evaluation" phases of chart functions
ClosedPublic

Authored by epriestley on Apr 19 2019, 1:12 AM.

Details

Summary

Depends on D20446. Currently, chart functions are both configured through arguments and evaluated through arguments. This sort of conflates things and makes some logic more difficult than it should be.

Instead:

  • Function arguments are used to configure function behavior. For example, scale(2) configures a function which does f(x) => 2 * x.
  • Evaluation is now separate, after configuration.

We can get rid of "sourceFunction" (which was basically marking one argument as "this is the thing that gets piped in" in a weird magical way) and "canEvaluate()" and "impulse".

Sequences of functions are achieved with compose(u, v, w), which configures a function f(x) => w(v(u(x))) (note order is left-to right, like piping x | u | v | w to produce y).

The new flow is:

  • Every chartable function is compose(...) at top level, and composes one or more functions. compose(x) is longhand for id(x). This just gives us a root/anchor node.
  • Figure out a domain, through various means.
  • Ask the function for a list of good input X values in that domain. This lets function chains which include a "fact" with distinct datapoints tell us that we should evaluate those datapoints.
  • Pipe those X values through the function.
  • We get Y values out.
  • Draw those points.

Also:

  • Adds accumluate().
  • Adds sum(), which is now easy to implement.
  • Adds compose().
  • All functions can now always evaluate everywhere, they just return null if they are not defined at a given X.
  • Adds repeatable arguments for compose(f, g, ...) and sum(f, g, ...).
Test Plan

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Apr 19 2019, 1:12 AM
epriestley requested review of this revision.Apr 19 2019, 1:14 AM
epriestley added inline comments.Apr 19 2019, 1:25 AM
src/applications/fact/chart/PhabricatorAccumulateChartFunction.php
39

This is when you x | accumulate and numerically approximate the integral of some pure math function. It "works" (produces a meaningful -- but possibly inaccurate -- integral using crude numerical methods) and letting you do this means I don't have to classify functions anymore, so I'm just allowing this for now.

If this is too confusing we could stop it later, but I don't think users are going to see this stuff.

src/applications/fact/chart/PhabricatorChartFunction.php
6–7

The axis stuff will come back at some point, it just isn't used right now.

src/applications/fact/chart/PhabricatorChartFunctionChain.php
37–40 ↗(On Diff #48805)

This is the x | ... | ... bit, which is now just a mundane loop.

src/applications/fact/chart/PhabricatorSumChartFunction.php
11–16

At some point this could take "1 or more functions", there's just no argument parsing support for repeatable arguments yet.

src/applications/fact/controller/PhabricatorFactChartController.php
51–75

This is pretty hard to read as a PHP specification, but I'm showing that these two chains are now equivalent:

x | sum(fact(tasks-open), fact(tasks-all)) | accumulate
x | sum(accumulate(fact(tasks-open)), accumulate(fact(tasks-all))))

That is, A(X + Y) is the same as A(X) + A(Y). In the screenshot, they produce the same datapoints and draw on top of one another. This is heartening.

epriestley planned changes to this revision.Apr 19 2019, 1:49 PM

Sleeping on this, "FunctionChain" should just be a function chain(...) or compose(...) or something, which evaluates as compose(f, g, h)(x) => f(g(h(x))).

Let me get somewhere useful with this stuff and then I'll go back and break it up a bit.

epriestley updated this revision to Diff 48859.Apr 29 2019, 3:58 PM
  • More pure variation that does everything we need with just functions.
epriestley planned changes to this revision.Apr 29 2019, 4:01 PM

This variation feels much better, but let me get somewhere useful with it to make sure it isn't another blind alley.

epriestley retitled this revision from Treat chart functions as pipe/chain, instead of both configuring and evaluating through function composition to Separate the "configuration" and "evaluation" phases of chart functions.Apr 29 2019, 4:05 PM
epriestley edited the summary of this revision. (Show Details)
epriestley edited the test plan for this revision. (Show Details)
epriestley requested review of this revision.Apr 30 2019, 3:28 PM

See followups; this seems to at least get us somewhere.

amckinley accepted this revision.May 17 2019, 4:07 PM
amckinley added inline comments.
src/applications/fact/chart/PhabricatorAccumulateChartFunction.php
53

"is the largest"

This revision is now accepted and ready to land.May 17 2019, 4:07 PM