Page MenuHomePhabricator

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

Authored by epriestley on Apr 19 2019, 1:12 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Dec 15, 2:28 PM
Unknown Object (File)
Fri, Dec 6, 1:46 AM
Unknown Object (File)
Wed, Dec 4, 6:06 PM
Unknown Object (File)
Sun, Dec 1, 10:13 PM
Unknown Object (File)
Nov 26 2024, 8:46 AM
Unknown Object (File)
Nov 26 2024, 6:31 AM
Unknown Object (File)
Nov 25 2024, 4:00 AM
Unknown Object (File)
Nov 21 2024, 8:39 AM
Subscribers

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

Screen Shot 2019-04-29 at 8.57.14 AM.png (1×1 px, 375 KB)

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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.

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.

  • More pure variation that does everything we need with just functions.

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)

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

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