2

I tried to modify standard sorting approach and add certain randomness to sorting Less interface. when

if (u[i] - u[j]) <= 0

or

if u[i] < u[j]

it works as expected But

if (u[i] - u[j]) <= rv

condition produces panic after several executions

package main
import ( 
    "crypto/rand"
    "fmt"
    "math/big"
    "sort"
)

type FuzzySorter []float64

func (u FuzzySorter) Len() int {
    return len(u)
}
func (u FuzzySorter) Swap(i, j int) {
    u[i], u[j] = u[j], u[i]
}
func (u FuzzySorter) Less(i, j int) bool {
    pom, _ := rand.Int(rand.Reader, big.NewInt(int64(2)))
    rv := float64(pom.Int64())
    if (u[i] - u[j]) <= rv {
        return true
    } else {
        return false
    }

}
func (u FuzzySorter) Sort() FuzzySorter {
    sort.Sort(u)
    return u
}

func main() {
    unsorted := FuzzySorter{
        0,
        1,
        1,
        1,
        1,
        6,
        0,
        4,
        6,
        1,
        1,
        1,
        0,
        2,
        8,
        1,
        5,
        4,
        6,
        6,
        6,
        16,
        12,
        6,
        1,
        1,
        1,
        0,
        0,
        11,
        2,
        14,
        16,
        6,
        12,
        0,
        4,
        1,
        0,
        16,
        2,
        6,
        0,
        0,
        0,
        0,
        1,
        11,
        1,
        0,
        2,
        1,
        1,
        1,
        1,
        0,
        1,
        12,
        10,
        1,
        5,
        2,
        6,
        4,
        1,
        0,
        0,
        11,
        1,
        1,
        2,
        2,
        1,
        0,
        0,
        1,
        0,
        1,
        17,
        2,
        1,
        1,
        2,
        0,
        3,
        7,
        1,
        5,
        1,
        0,
        1,
        0,
        0,
        0,
        1,
        3,
        1,
        1,
        1,
        2,
        1,
        0,
        3,
        1,
        6,
        1,
        1,
        0,
        1,
        12,
        0,
        1,
        1,
        0,
        1,
        0,
        0,
        6,
        1,
        2,
        2,
        0,
        0,
        2,
        1,
        1,
        0,
        4,
        4,
        1,
        1,
        1,
        0,
        1,
        1,
        1,
        2,
        0,
        0,
        1,
        0,
        1,
        2,
        1,
        2,
        1,
        1,
        0,
        0,
        4,
        1,
        0,
        1,
        0,
        1,
        1,
        3,
        1,
        0,
    }
    unsorted.Sort()
    fmt.Println(unsorted)

}

https://play.golang.org/p/4AxNRN4VD7

panic message

panic: runtime error: index out of range

goroutine 1 [running]:
panic(0x176ba0, 0x1040a010)
    /usr/local/go/src/runtime/panic.go:464 +0x700
main.FuzzySorter.Less(0x10456000, 0x9f, 0x9f, 0x19, 0xffffffff, 0x4, 0x1, 0xd)
    /tmp/sandbox201242525/main.go:21 +0x140
main.(*FuzzySorter).Less(0x10434140, 0x19, 0xffffffff, 0x5c, 0x1, 0x10434140)
    <autogenerated>:3 +0xc0
sort.doPivot(0xfef741b0, 0x10434140, 0x19, 0x9f, 0x7, 0x19)
    /usr/local/go/src/sort/sort.go:128 +0x280
sort.quickSort(0xfef741b0, 0x10434140, 0x19, 0x9f, 0xe, 0xfef741b0)
    /usr/local/go/src/sort/sort.go:195 +0xa0
sort.Sort(0xfef741b0, 0x10434140)
    /usr/local/go/src/sort/sort.go:229 +0x80
main.FuzzySorter.Sort(0x10456000, 0x9f, 0x9f, 0x1, 0x0, 0x0, 0x0, 0x1777a0)
    /tmp/sandbox201242525/main.go:29 +0xa0
main.main()
    /tmp/sandbox201242525/main.go:195 +0xc0
Y01rY5Ogfl
  • 41
  • 1
  • 1
  • 8
  • 4
    The Less method must be deterministic, i.e. calling Less for the same 2 values must always produce the same result. – JimB Apr 12 '16 at 15:46
  • 3
    Fuzz the output instead of modifying the less function. – Charlie Tumahai Apr 12 '16 at 15:51
  • Philosophically, I completely agree. Technically, do you mean that that will be enforced in some way? Ie - are you saying that having a nondeterministic `Less()` will/can cause a panic? – Brian Gerard Apr 12 '16 at 15:56
  • I just port my function from perls builtin sort to golangs builtin sort and surprised a bit. There is no official info about deterministic/nondeterministic requirements in "sort" godoc. – Y01rY5Ogfl Apr 12 '16 at 16:01
  • 2
    @BrianGerard Yes, the [code](https://golang.org/src/sort/sort.go#L112) assumes that the ordering is deterministic. – Charlie Tumahai Apr 12 '16 at 16:21
  • 1
    @Y01rY5Ogfl: a randomized sort comparator is the incorrect way to add randomness. This introduces unintended bias (see https://bost.ocks.org/mike/shuffle/compare.html for a good visualization) and possibly (as seen here) completely unexpected behavior. – JimB Apr 12 '16 at 16:21
  • Your code snippet doesn't panicking neither on my machine nor on playground. Can you present some reproducible case or your full panic message? – Uvelichitel Apr 12 '16 at 17:51
  • @Uvelichitel you need execute code several times it panics both on machine and playground , i'v added full panic message to body of initial post. If you rerun code in playground several times >3 i think, don't forget to add extra space to code in order to prevent playground caching – Y01rY5Ogfl Apr 12 '16 at 18:00

2 Answers2

0

As far as I can understand Go sort implementation it expect two negative comparisons eg. Less(i, j) and Less(j, i) both return false, which it treats as equality, but not positive. E.g Less(i, j) and Less(j, i) can't both return true. So you can easily achieve desired result logically correct and deterministic way, just

if (u[i] - u[j]) < -1 {
        return true
    } else {
        return false
    }

https://play.golang.org/p/VcKI9uzcM9

Uvelichitel
  • 8,220
  • 1
  • 19
  • 36
0

As of Go 1.8, there is an easier way to sort a slice that does not require you to define new types. You simply create a Less (anonymous) lambda.

a := []int{5, 3, 4, 7, 8, 9}
sort.Slice(a, func(i, j int) bool {
    return a[i] < a[j]
})
for _, v := range a {
    fmt.Println(v)
}

This will sort in ascending order, if you want the opposite, simply write a[i] < a[j]

Franck Jeannin
  • 6,107
  • 3
  • 21
  • 33