[BUG] Padding.pad drops a data element, and the end padding is backwards

[marklam]

Describe the bug FSharp.Stats.Signal.Padding.pad loses the last data point, and the end padding is reversed.

If padding (X,100) .. (Y-1,199); (Y,200) with 3 zero-valued items and a minDistance of 1.0 you should get:

(X-3,0); (X-2,0); (X-1,0); (X,100) ..  (Y-1,199); (Y,200); (Y+1,0); (Y+2,0); (Y+3,0)

but you actually get

(X-3,0); (X-2,0); (X-1,0); (X,100) .. (Y-1,199); (Y+3,0); (Y+2,0); (Y+1,0)

so (Y,200) is lost, and the padding elements at the end are reversed.

To Reproduce

#r "nuget:FSharp.Stats"
#r "nuget:Expecto"

open System
open FSharp.Stats.Signal
open Expecto

let dataLength = 20
let padding = 10

let data =
    Array.init dataLength (
        fun i ->
            (3.0 + float i, 7.0 - float i)
    )

let expectLeadIn  = Array.init padding (fun i -> (3.0 - float (padding-i), 0.0))
let expectLeadOut = Array.init padding (fun i -> (3.0 + float (dataLength + i), 0.0))
let expectedPadded = Array.concat [expectLeadIn; data; expectLeadOut]

let padded = Padding.pad data 1.0 Double.PositiveInfinity (-) (+) padding Padding.BorderPaddingMethod.Zero Padding.InternalPaddingMethod.NaN Padding.HugeGapPaddingMethod.NaN

Expect.equal (Array.sub padded 0 padding) expectLeadIn
Expect.equal (Array.sub padded (padded.Length - padding) padding) expectLeadOut
Expect.equal (Array.sub padded padding data.Length) data "All the original data should be contained in the padded data"
Expect.equal padded.Length (data.Length + 2 * padding) "Length should be the original data length plus padding at each end"
Expect.equal (padded |> Array.sortBy fst) expectedPadded "Result should be the lead-in, whole data, then lead-out (maybe not in order?)"
Expect.equal padded expectedPadded "Result should be the lead-in, whole data, then lead-out"

Expected behavior The padded data should have all the original data, with the same number of padding elements at each end. The fst of each padding data element should increase by minDistance.

Screenshots If applicable, add screenshots to help explain your problem.

OS and framework information (please complete the following information):

• OS: [e.g. Windows 10]
• OS Version [e.g. 1809]
• .Net core SDK version [e.g. 2.2.101 ]

Additional context Add any other context about the problem here.

[bvenn]

Thanks for reporting!:rocket: I've traced down the issue to the following two lines:

The Array.rev must be removed at: https://github.com/fslaborg/FSharp.Stats/blob/3d6a2201c45d792d95cc127c97c377fa4bcf496c/src/FSharp.Stats/Signal/Padding.fs#L140-L144

The last point has to be integrated here: https://github.com/fslaborg/FSharp.Stats/blob/3d6a2201c45d792d95cc127c97c377fa4bcf496c/src/FSharp.Stats/Signal/Padding.fs#L164-L167

After these modifications the result looks as follows:

//#r "nuget:FSharp.Stats"
#r @"C:\Users\bvenn\source\repos\FSharp.Stats\src\FSharp.Stats\bin\Release\netstandard2.0\FSharp.Stats.dll"
#r "nuget:Expecto"
#r "nuget: Plotly.NET"

open Plotly.NET

open System
open FSharp.Stats.Signal
open Expecto

let dataLength = 20
let padding = 10

let data =
    Array.init dataLength (
        fun i ->
            (3.0 + float i, 7.0 - float i)
    )

let expectLeadIn  = Array.init padding (fun i -> (3.0 - float (padding-i), 0.0))
let expectLeadOut = Array.init padding (fun i -> (3.0 + float (dataLength + i), 0.0))
let expectedPadded = Array.concat [expectLeadIn; data; expectLeadOut]

let padded = 
    Padding.pad data 1.0 Double.PositiveInfinity (-) (+) padding Padding.BorderPaddingMethod.Zero Padding.InternalPaddingMethod.NaN Padding.HugeGapPaddingMethod.NaN

let indices = Array.init padded.Length (fun x -> string x)

[
Chart.Point(data,Name="raw data")           |> Chart.withMarkerStyle (Size=10)
Chart.Point(expectedPadded,Name="expected") |> Chart.withMarkerStyle (Size=8)
Chart.Point(padded,Name="actual",MultiText=indices,TextPosition=StyleParam.TextPosition.TopRight)           |> Chart.withMarkerStyle (Size=6)
]
|> Chart.combine
|> Chart.show

@marklam, if it is ok for you I would integrate your tests into the test project. Of course you can add them yourself and file a PR if you want to.

[marklam]

@bvenn I'm more than happy for you to integrate those tests. Thanks for responding so quickly!

[bvenn]

closed by 153d95ed3507018326fbb86b707b949a7b741e38