I have written an algorithm is Swift for finding the maximum value and its index in a Swift array. This is inspired by the "max.m" function in Matlab & Octave.

Could the experts here suggest a way to improve this algorithm in terms of speed? I mean could it be made faster or you think this is a reasonable approach for large arrays (sometimes 15000 samples).

public func max (y: [Double]) -> (Int, Double) {

let inLen = y.count

var out = Double()
var outp = Int()

if (1 == inLen) { // if only one element
    out = y[0]
    outp = 0
} else if (0 == inLen) { // if no elements
    out = -1
    outp = -1
} else {
    out = y[0]
    outp = 0
    for ii in 1...inLen-1 {
        if (out<y[ii]){
            out = y[ii]
            outp = ii
        }
    }
}
return (outp, out)
}

// Call the function

let y: [Double] = [3, 4, 5, 6, 7, 8, 9, 100, 100, 11, 12, 13, 14, 15, -8, -7, -7, 99]
let (ind, value) = max(y: y)
print(ind) // 7
print(value) // 100.0

回答1:

You can use the vDSP_maxviD)() function from the Accelerate framework. The vDSP functions use vDSP_Length (aka UInt) for array counts and indices, so you have to convert the index to an Int for Swift interoperability.

import Accelerate

let array: [Double] = ...

var elem = 0.0
var vdspIndex: vDSP_Length = 0
vDSP_maxviD(array, 1, &elem, &vdspIndex, vDSP_Length(array.count))
let idx = Int(vdspIndex)

print("max:", elem, "at index:", idx)

This turned out to be about a factor 5 faster than your explicit loop for a 15,000 element array (on an iMac compiled in Release mode).

回答2:

Code Review on the original code

Here are the changes I would make. All this is kind of rough (I haven't checked if all these compile), but will put you on the right track.

Make the code generic

public func max (y: [Double]) -> (Int, Double) {

becomes:

public func max<T: Comparable>(y: [T]) -> (Int, T) {

Remove the useless parameter keyword y, and rename y to something meaningful
```
public func max<T: Comparable>(_ array: [T]) -> (Int, T) {
```

Add keywords to the result:

public func max<T: Comparable>(_ array: [T]) -> (index: Int, value: T) {

Add it as an extension on Array or RandomAccessCollection, replacing the need for the parameter entirely:

extension Array where Element: Comparable {
    public func max() -> (index: Int, value: Element) {
        // ...
    }
}

Inline the unnecessary variable inLen, or at least name it better, like just count
Rename out and outp to something better, such as maxValue and maxIndex
Don't use yoda comparisons in Swift. = isn't an exprission in Swift, so there's no risk of it accidentally causing assignment in your if statement rather than comparison. It would trigger a compiler error. Also, omit the
```
if (1 == count) {
```
should be
```
if (count == 1) {
```
Omit uneeded parathesis in the if statements:
```
if count == 1 {
```
Rearrange the count checks in a more logical order. Rather than 1, 0, 1+, order it as 0, 1, 1+.

Return early, rather than using if/elseif/else to skip chunks of code to get to a common return. Instead of:

extension Array where Element: Comparable {
    public func max() -> (index: Int, value: Element) {
        var maxValue = Double()
        var maxIndex = Int()

        if count == 0 { // if no elements
            maxValue = -1
            maxIndex = -1
        } else if count == 1 { // if only one element
            maxValue = self[0]
            maxIndex = 0
        } else {
            maxValue = self[0]
            maxIndex = 0
            for i in 1...inLen-1 {
                if (maxValue < self[i]){
                    maxValue = self[i]
                    maxIndex = i
                }
            }
        }
        return (index: maxIndex, value: maxValue)
    }
}

try this:

extension Array where Element: Comparable {
    public func max() -> (index: Int, value: Element)? {
        var maxValue = Double()
        var maxIndex = Int()

        if count == 0 { return (index: -1, value: -1) } 
        if count == 1 { return (index: 0, value: self[0]) }

        maxValue = self[0]
        maxIndex = 0
        for i in 1...count-1 {
            if (maxValue < self[i]) {
                maxValue = self[i]
                maxIndex = i
            }
        }
        return (index: maxIndex, value: maxValue)
    }
}

Now you can remove the forward declarations of maxValue and maxIndex

extension Array where Element: Comparable {
    public func max() -> (index: Int, value: Element) {
        if count == 0 { return (index: -1, value: -1) } 
        if count == 1 { return (index: 0, value: self[0]) }

        var maxValue = self[0]
        var maxIndex = 0
        for i in 1...count-1 {
            if (maxValue < self[i]) {
                maxValue = self[i]
                maxIndex = i
            }
        }
        return (index: maxIndex, value: maxValue)
    }
}

Avoid rewriting things like 1 ... x-1 instead, use ..<: for i in 1 ..< count {
In this case, it's even better to use self.indices, which achieves the same thing:
```
for i in self.indicies {
```
If you need both the indices, and the values assosiated with those indices, use enumerated():
```
for (index, value) in self.enumerated() {
```

Never use sentinel values like -1 and "" in Swift. We have optionals for expressing the absense of a value. Use them:

extension Array where Element: Comparable {
    public func max() -> (index: Int, value: Element)? {
        if count == 0 { return nil } 
        if count == 1 { return (index: 0, value: self[0]) }

        var maxValue = self[0]
        var maxIndex = 0
        for (index, value) in self.enumerated() {
            if (maxValue < value) {
                maxValue = value
                maxIndex = index
            }
        }
        return (index: maxIndex, value: maxValue)
    }
}

I would also use tuple assignment to make a bit shorter:

extension Array where Element: Comparable {
    public func max() -> (index: Int, value: Element)? {
        if count == 0 { return nil } 
        if count == 1 { return (index: 0, value: self[0]) }

        var (maxIndex, maxValue) = (0, self[0])
        for (index, value) in self.enumerated() {
            if (maxValue < value) {
                (maxIndex, maxValue) = (index, value)
            }
        }
        return (index: maxIndex, value: maxValue)
    }
}

Now that we're using tuple assignment, we can see that we can just combine maxValue and maxIndex into a tuple, which we return directly:

extension Array where Element: Comparable {
    public func max() -> (index: Int, value: Element)? {
        if count == 0 { return nil } 
        if count == 1 { return (index: 0, value: self[0]) }

        var maxElement = (index: 0, value: self[0])
        for (index, value) in self.enumerated() {
            if (maxElement.value < value) { maxElement = (index, value) }
        }
        return maxElement
    }
}

Here's how to call the new method:

let array: [Double] = [3, 4, 5, 6, 7, 8, 9, 100, 100, 11, 12, 13, 14, 15, -8, -7, -7, 99]

if let (maxIndex, maxValue) = array.max() {
    print("The max element is \(maxValue) at index \(maxIndex)")
}
else {
    print("The array is empty, and has no max element or index.")
}

The idomatic Swift approach

let array: [Double] = [3, 4, 5, 6, 7, 8, 9, 100, 100, 11, 12, 13, 14, 15, -8, -7, -7, 99]

if let (maxIndex, maxValue) = array.enumerated.max{ $0.element < $1.element } {
    print("The max element is \(maxValue) at index \(maxIndex)")
}
else {
    print("The array is empty, and has no max element or index.")
}

MartinR's approach, improved

Here's a wrapper around MartinR's approach, to make it easier to integrate with other Swift code:

func max(of array: [Double]) -> (index: Int, value: Double)? {
    var maxValue = Double()
    var maxIndex = vDSP_Length()
    vDSP_maxviD(array, 1, &maxValue, &maxIndex, vDSP_Length(array.count))

    if maxValue == -Double.infinity { return nil }

    return (index: Int(maxIndex), value: maxValue)
}