argmax of List of Double

Question

Here is my implementation of arg max in Scala. q_table_test.txt contains:

["---------,'0','0','0','0','0','0','0','0','0', "X--------,'0.', '0.1', '0.1', '0.1', '0.1', '0.1', '0.1', '0.1', '0.1'"]

For example the String ---------,'0','0','0','0','0','0','0','0','0' maps to key: "---------" , with values: [0,0,0,0,0,0,0,0,0] After filtering values which are not valid I return the arg max of values from the list of positions.

Coming from a java background, what is a more functional programming principled method of implementing arg max?

Should the list filtering be contained in a separate function?

The QTable apply() method parses reads a file and returns Map[String , List[Double]] . It is evaluated lazily as it should just be evaluated once.

I think the use of apply in lazy val is not correct ?

Is this a 'good' method of finding the arg max value ?:

val qValues = bindings.get(state).getOrElse(List.fill(8)(0.0)).zipWithIndex
val availableQValueBoardPositions = qValues.filter(f => remainingPositions.contains(f._2))

Complete code:

import java.io.InputStream
import play.api.Environment

case class QTable(bindings: Map[String, List[Double]]) {

  def getArgMaxValue(state: String, remainingPositions: List[Int]) = {
    val qValues = bindings.get(state).getOrElse(List.fill(8)(0.0)).zipWithIndex
    val availableQValueBoardPositions = qValues.filter(f => remainingPositions.contains(f._2))
    availableQValueBoardPositions.maxBy(x => x._1)._2
  }

}

object QTable {

  private def getQTableFromFile(filename: String) = {
    lazy val env = Environment.simple()
    lazy val is: InputStream = Option(env.classLoader.getResourceAsStream(filename)).get
    scala.io.Source.fromInputStream(is).mkString
  }

  private def cleanData(data: String): String = {
    data.replace("\"", "").replace("\'", "").replace("[", "").replace("]", "")
  }

  def apply(filename: String) = {

    val number_attributes_per_instance = 10;

    val qtable = getQTableFromFile(filename)

    lazy val cleanedQtable: String = cleanData(qtable)
    lazy val dataInstances: List[List[String]] = cleanedQtable.split(",").toList.grouped(number_attributes_per_instance).toList
    lazy val stateIdValues: List[String] = dataInstances.map(m => m.head.trim)
    lazy val stateAttributeValues: List[List[Double]] = dataInstances.map(m => m.tail.map(x => x.toDouble))
    lazy val policy: QTable = new QTable(stateIdValues.zip(stateAttributeValues).toMap)

    policy
  }

}

object ArgMain extends App {
  print(QTable("q_table_test.txt").getArgMaxValue("---------", List(1, 2, 3)))
}

Answer 1

So you want the index of the maximum value in your collection. That's a lot of code for such a simple task.

Let's start at the top.

q_table_test.txt - The sample data supplied is rather ridiculous. The elements that will become "attribute values" are all the same. How do you know the code is correct if there is no maximum? Does it matter which index is returned?

QTable.apply(filename) - The number_attributes_per_instance should be a configuration parameter, or derived directly from the input data. What if you needed to support more than one number_attributes_per_instance?

You don't appear to know what a lazy val is and what it's for. Its usage here makes no sense.

There are no safety checks. The file text is loaded as if formatting errors can't happen.

getQTableFromFile() - Why is getResourceAsStream() wrapped in an Option and then unwrapped with a .get? That's like loaning your brother a dollar so that he can repay the dollar he borrowed from you yesterday. A lot of busy work that accomplishes nothing.

cleanData() - Very inefficient.

getArgMaxValue() - If the state parameter is faulty you go through the trouble of building a bogus qValues list and then the elaborate and convoluted steps of finding the index of its max value, of which there is none. If you're going to lie to the user why not just return remainingPositions.head and be done with it?

---

As a demonstration, here's a smaller yet safer implementation of the same basic outline.

import scala.util.{Try, Using}  //Scala 2.13.x

class QTable(bindings: Map[String, Array[Double]]) {
  def getMaxValueIdx(state              : String
                    ,remainingPositions : List[Int]
                    ) :Option[Int] =
    bindings.get(state)
            .flatMap(arr => remainingPositions.maxByOption(arr.lift))
}

object QTable {
  private def loadFromFile(filename :String) :Try[String] =
    Using(scala.io.Source.fromFile(filename)) {
      _.getLines().map(_.takeWhile(_ != '#').trim).mkString
    }

  private val recFormat =
    """([^,"\[\]]+)\s*,((\s*('\d*\.?\d*')\s*,?)+)""".r.unanchored

  def apply(filename :String) :QTable =
    loadFromFile(filename).flatMap{ data => Try {
      recFormat.findAllMatchIn(data)
        .map(m => m.group(1) -> m.group(2)
                                 .split("[,\\s]+")
                                 .collect{case s"'$n'" => n.toDouble}
            ).toMap
    }}.fold(err => {println(err); new QTable(Map.empty)}
           ,new QTable(_))
}

My q_table_test.txt file:

[
"1st 9 Tbl,'0','1','2','3','4','5','6','7','8', #table of 9 elements
"Short Tbl,   '7' , '1.1','.22', '3'  ,'12.0' , #mixed spacing
"Split Tbl, '17', '1.25', '.127', '300.003'     #1st part
          , '12.0', '4321', '.1234'           , #2nd part
"2nd 9 Tbl,'0.','0.9','0.8','0.7','0.6','0.5','0.4','0.3','0.2'"
]

testing:

val qx = QTable("bogus.txt")    // (No such file or directory)
qx.getMaxValueIdx("1st 9 Tbl", List(7, 2, 3))  // None

val qt = QTable("./q_table_test.txt")
qt.getMaxValueIdx("1st 9 Tbl", List(7, 2, 3))  // Some(7)
qt.getMaxValueIdx("1st 9 Tbl", List())         // None
qt.getMaxValueIdx("Short Tbl", List(2,3,0))    // Some(0)
qt.getMaxValueIdx("Short Tbl", List(2,3,990))  // Some(3)
qt.getMaxValueIdx("NoSuchTbl", List(5,0,8))    // None
qt.getMaxValueIdx("Split Tbl", List(5,0,8))    // Some(5)
qt.getMaxValueIdx("2nd 9 Tbl", List(7, 2, 3))  // Some(2)