You're absolutely correct, but I figured it wasn't important because of the
immediate sortBy. I went ahead and changed the implementation to use a
Vector instead of a List to preserve the same ordering as the older
implementation (also tested it to verify results were the same). Append
operations for Vector instances are effectively constant, whereas for List
instances they are Linear (
http://docs.scala-lang.org/overviews/collections/performance-characteristics.html).
I also fixed up the formatting.


On Sat, Jul 6, 2013 at 4:40 PM, Mark Hamstra <notifications@github.com>wrote:

> In mllib/src/main/scala/spark/mllib/regression/RidgeRegression.scala:
>
> > -      val highValue = crossValidate((high - mid) / 2 + mid)
> > -      val (newLow, newHigh) = if (lowValue._2 < highValue._2) {
> > -        (low, mid + (high-low)/4)
> > -      } else {
> > -        (mid - (high-low)/4, high)
> > -      }
> > -      if (newHigh - newLow > 1.0E-7) {
> > -        // :: is list prepend in Scala.
> > -        lowValue :: highValue :: binSearch(newLow, newHigh)
> > -      } else {
> > -        List(lowValue, highValue)
> > +      @tailrec
> > +      def loop(
> > +        low: Double,
> > +        high: Double,
> > +        acc: List[(Double, Double, DoubleMatrix)]): List[(Double, Double, DoubleMatrix)] = {
>
> Does it matter? The only present caller of binSearch does a sortBy
> immediately? If it does matter, there are, of course, trivial fixes.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/mesos/spark/pull/679/files#r5053678>
> .
>