beeminder / road

Beebrain and Visual Graph Editor
http://graph.beeminder.com
Other
13 stars 3 forks source link

Aggday and where the red derail/rerail triangle is drawn #59

Open bsoule opened 5 years ago

bsoule commented 5 years ago
### Desiderata
- [ ] Rewrite description below in light of discussion/conclusions in the comments

This problem goes back at least as far as the Python implementation of Beebrain.

User-Bethany: I derailed my weight goal on the 28th. I.e., my weight on the 27th was above the red line. The rerail stuff picked the correct value for the #DERAIL (nee "RECOMMITTED") datapoint and moved the red line correctly to 68kg, the min of the datapoints on the 27th. However, on the 28th itself, in addition to the weights I registered, Withings mis-assigned one of Danny's weights to my account -- the 70.3 that's labeled "ambiguous" in the comments.

The red triangle that's drawn to indicate the derail is pointing at that datapoint, not the derail value or the #DERAIL datapoint. (Possibly because it's the max on the 28th? Or maybe just because it's the last datapoint on the 28th?)

The red triangle should point at the derailing value (or at the derail/rerail point). Definitely confusing/weird that it's pointing at that 70.3 value.

weight___b_weight_goal_page

I put the bb file in the quals/automon suite as wrong-recommit-triangle.bb

Cognata

Verbata: aggday, graph legend, graph aesthetics, bbserver, jsbrain, derail triangle, derail indicator,

saranli commented 5 years ago

Yes, this is indeed due to the erroneous value being considered as the "worst value" for that day. Here is the relevant code snippet:

  // Adjust derailment markers to indicate worst value for that day
  for (i = 0; i < derails.length; i++) {
    ct = derails[i][0]+bu.SID
    if (worstval.hasOwnProperty(ct)) derails[i][1] = worstval[ct]
  }

Since the erroneous datapoint is specific to this goal and jsbrain has no way of knowing that the value was erroneous, I think this can be safely closed, unless you think we should pick the derailment value rather than the worst value to show for the derailment?

bsoule commented 5 years ago

Is it on purpose that we're inventing a new metric ("worst for the day") and using it rather than using the aggday'd value here? Using the aggday'd value feels more correct to me.

Actually, I just double checked and for Do More goals we use the aggday value, not the worst-for-the-day. See here:

elfthing___b_elfthing_goal_page
saranli commented 5 years ago

No idea. I will defer to dreev for this since I faithfully ported from the pybrain code. The relevant computation is as follows:

          let vw = allvals[ct].map(e=>e[0])
          worstval[ct] = (goal.yaw<0)?bu.arrMax(vw):bu.arrMin(vw)

basically computes the min or max for all days with datapoints including (I think), the aggregated value. I agree that this probably needs more thought. Perhaps subscribe dreev to this?

dreeves commented 5 years ago

Uluc is right. But there was some reason I went with worstval instead of aggval. Python code for comparison:

worstval[t] = (max(allvals[t]) if yaw<0 else min(allvals[t]))

Fortunately, cmonitor [now called automon] makes this so nice to get to the bottom of! I can try aggval instead and see what all changes in the testsuite...

PS: Relevant commit: https://github.com/beeminder/road/commit/a293ec8b5367234f1e2dc88d0cc871340a992908

dreeves commented 5 years ago

To do this really right we need to compute the aggregated datapoint as of the #DERAIL (nee 'RECOMMITTED') datapoint, when the derail happened. bmndr.co/changelog#3157