Stabilize ASAP #498
Stabilize ASAP #498
Conversation
WireBaron
requested review from
rtwalker,
JLockerman,
thatzopoulos,
davidkohn88 and
epgts
WireBaron
force-pushed
the
br/asap_stable
branch
2 times, most recently
from
3d05ae8
to
cb8eaea
Compare
| (points.last().unwrap().ts - points.first().unwrap().ts) / points.len() as i64 | ||
| }; | ||
| let mut normal = downsample_and_gapfill_to_normal_form(&points, downsample_interval); | ||
|
|
||
| let start_ts = points.first().unwrap().ts; |
There was a problem hiding this comment.
Can this happen from user error alone? If so, we ought to provide an error message by changing .unwrap() to .expect("points must not be empty") or something. (Same question for another unwrap below in asap_on_timevector)
Is it possible for end_ts to be the same as start_ts and does that matter?
There was a problem hiding this comment.
This should be impossible to hit during normal use. We exit this function right away if called on None and won't create a Some unless we have at least on value. Similarly we need a time/value pair to create a timevector. If the data is only one point we should just return that point, and, now that I just fixed the div0 we were hitting in that case, we do so.
WireBaron
force-pushed
the
br/asap_stable
branch
from
cb8eaea
to
5a72451
Compare
|
bors r+ |
498: Stabilize ASAP r=WireBaron a=WireBaron This PR moves the ASAP implementation out of toolkit_experimental. It also removes some of the odd behavior where we were trying to normalize the incoming data points before applying the algorithm (users should do this themselves if this is the behavior they desire). As a result, we should now correctly match the reference implementation exactly, and this change also adds a PR to that effect. Co-authored-by: Brian Rowe <[email protected]>
|
bors r- |
|
Canceled. |
WireBaron
force-pushed
the
br/asap_stable
branch
from
5a72451
to
cffd866
Compare
|
bors r+ |
|
Build succeeded: |
This PR moves the ASAP implementation out of toolkit_experimental. It also removes some of the odd behavior where we were trying to normalize the incoming data points before applying the algorithm (users should do this themselves if this is the behavior they desire). As a result, we should now correctly match the reference implementation exactly, and this change also adds a PR to that effect.