Ising Model - Atom Vibration Simulation in Scala

I am an experienced Java developer (12+ years) and have recently switched to Scala and I love it. However I feel not comfy yet and I have a feeling that I might use to many paradigms from the good old Java days.

That's why I would like to start with some simple code I wrote while viewing a Stanford Seminar on Youtube with the Topic "Deep Learning for Dummies". I tried to simulate the results presented in minute ~15:40.

Basically the code simulates vibration of atoms in the Ising Model in parallel, and gives a brief summary of how often a particular stable state has been reached.

Everything works as expected and it proves the numbers shown in the presentation.

No code is missing below in order to work and I did not use third party libs.

deeplearning/IsingModelSmall.scala

```
package deeplearning

import java.util.concurrent.atomic.AtomicInteger

import deeplearning.AtomState.AtomState
import deeplearning.AtomState._

/**
* Created by Julian Liebl on 25.11.15.
*
* All inspiration taken from youtu.be/hvIptUuUCdU. This class reproduces and proves the result shown in minute ~15:40.
*/
class IsingModelSmall {
case class MinMax(val min:Double, val max:Double)

val x1 = new Atom(Up) .fuse(-50 , new Atom(Up)) .fuse(99, new Atom(Up))
val x2 = new Atom(Down) .fuse(-50 , new Atom(Up)) .fuse(99, new Atom(Up))
val x3 = new Atom(Up) .fuse(-50 , new Atom(Down)) .fuse(99, new Atom(Up))
val x4 = new Atom(Up) .fuse(-50 , new Atom(Up)) .fuse(99, new Atom(Down))
val x5 = new Atom(Down) .fuse(-50 , new Atom(Down)) .fuse(99, new Atom(Up))
val x6 = new Atom(Up) .fuse(-50 , new Atom(Down)) .fuse(99, new Atom(Down))
val x7 = new Atom(Down) .fuse(-50 , new Atom(Up)) .fuse(99, new Atom(Down))
val x8 = new Atom(Down) .fuse(-50 , new Atom(Down)) .fuse(99, new Atom(Down))

/**
* Calculates the stable state of a Ising Model according to youtu.be/hvIptUuUCdU.
* It takes a random atom from the model as parameter and parses from there all atom

Thank you for this interesting question. Generally, your code is quite clean and the years of experience are visible through it. I'm also a Java developer, switching on Scala, that I love more and more.

Here are my observations about this code.

class Atom

The main thing about it is the usage of var connections. Variables are discouraged in Scala; they should be used in reduced scope without external exposure (connections has public access here).

So we can just transform the field into val:

val connections = ListBuffer[AtomConnection]()
// note that the type declaration with ":" is not necessary here

Now we cannot reassign the field, but we can operate on its contents when necessary. removeConection(Atom) method would look like this:

def removeConnection(atom : Atom): Unit ={
  connections.find(connection => connection.connectedAtom == atom) match {
    case None => {}
    case Some(atomConnection) => { removeConnection(atomConnection) }
  }
}

The principle is: when the first AtomConnection element is found that contains the specified Atom, it redirects to the overloaded removeConnection function with this element; otherwise it does nothing.

Please note that the equality is checked with ==, which is equivalent to Java's equals.

The same kind of changes can also be done with removeConnections(atoms:Seq[Atom]) function, but it doesn't seem to be used throughout the code.

The getConnections() function is not necessary at all. It looks like a sort of Java's residue; val connections already has public access and the reference is immutable.

Generally, I'm not sure about the validity of the choice to use var atomState and the exposed ListBuffer for connections field in Atom class. This sort of mutability is somewhat against Scala's principles, but I don't see yet how to work it around.

class IsingModelSmall

In def calcStableState, def getMinMaxWeight and def vibrateInner there are similar calls:

atom.connections.foreach(connection => {
  val connectedAtom = connection.connectedAtom
  ...
  if(!(touchedAtoms contains connectedAtom)) {
    ...
  }
}

They can be refactored into a dedicated function:

private def filterNonConnected(atom : Atom, touchedAtoms:Set[Atom]) =
  atom.connections.filter(connection =>
    !(touchedAtoms.contains(connection.connectedAtom))
  ).toList

Now the var sum counter in calcStableState can be eliminated:

def calcStableState(atom:Atom, touchedAtoms:Set[Atom] = Set()): Double ={
  val a1v = getAtomStateValue(atom.atomState)
  val sum = filterNonConnected(atom, touchedAtoms).foldLeft(0d)((sum, connection) => {
    val connectedAtom = connection.connectedAtom
    val a2v = getAtomStateValue(connectedAtom.atomState)
    sum + a1v * a2v * connection.weight + calcStableState(connectedAtom, touchedAtoms + atom)
  })
  -sum
}

What is done in calcStableState: 1) we filter the connections to work on with the refactored filterNonConnected; 2) we use the standard foldLeft function to calculate the sum. It takes the initial value 0d and performs the calculation using the current value in sum, tupled with each of the filtered connections.

getMinMaxWeight function can also be simplified using our refactored filterNonConnected and foldLeft:

def getMinMaxWeight(atom:Atom, touchedAtoms:Set[Atom] = Set()): MinMax ={
  filterNonConnected(atom, touchedAtoms).foldLeft(MinMax(0,0))((curMinMax, connection) => {
    val currentWeight = connection.weight
    val provisionalMinMax = getMinMaxWeight(connection.connectedAtom, touchedAtoms + atom)
    MinMax(List(curMinMax.min, currentWeight, provisionalMinMax.min).min,
           List(curMinMax.max, currentWeight, provisionalMinMax.max).max)
  })
}

Please note that there is not a single if-else left in this function.

BTW, there is a potential bug in the original implementation. MinMax shouldn't be initialized with (0, 0), but rather with (Double.MaxValue, Double.MinValue).

vibrateInner function can be refactored using the same principle, bu there will be two more things: 1) the removeConnection call needs to be separated into a dedicated loop, for example:

innerAtom.connections.foreach(connection => {
  connection.connectedAtom.removeConnection(innerAtom)
})

But I'm not sure that from the point of view of design it's a good solution, because it looks like a violation of LoD. There should be a better approach to solve it.

2) the calculation of connectedAtomState should be moved into a separate method, which will allow to eliminate the last var remaining in this part of code.

object IsingModelSmall

In Scala, there is no need to define def main(args : Array[String]). A simple extension of App would do the job:

object IsingModelSmall extends App {
  // the body of the main(args) method to be placed here directly
}

StackExchange Code Review Q#112068, answer score: 5