Skip to content

Commit

Permalink
Improving type evaluation and lookup (#2807)
Browse files Browse the repository at this point in the history
Fixing type evaluation problems
  • Loading branch information
orodeh authored and cjllanwarne committed Nov 9, 2017
1 parent 9942d52 commit 6b203f9
Show file tree
Hide file tree
Showing 4 changed files with 187 additions and 10 deletions.
6 changes: 4 additions & 2 deletions wdl/src/main/scala/wdl/WdlNamespace.scala
Original file line number Diff line number Diff line change
Expand Up @@ -468,13 +468,15 @@ object WdlNamespace {
invalidVariableReferences ++ typeMismatches
}

private def lookupType(from: Scope)(n: String): WomType = {
private [wdl] def lookupType(from: Scope)(n: String): WomType = {
val resolved = from.resolveVariable(n)
resolved match {
case Some(d: DeclarationInterface) => d.relativeWdlType(from)
case Some(c: WdlCall) => WdlCallOutputsObjectType(c)
case Some(s: Scatter) => s.collection.evaluateType(lookupType(s), new WdlStandardLibraryFunctionsType, Option(from)) match {
case Success(a: WomArrayType) => a.memberType
case Success(WomArrayType(aType)) => aType
// We don't need to check for a WOM map type, because
// of the custom unapply in object WomArrayType
case _ => throw new VariableLookupException(s"Variable $n references a scatter block ${s.fullyQualifiedName}, but the collection does not evaluate to an array")
}
case Some(_: WdlNamespace) => WdlNamespaceType
Expand Down
12 changes: 7 additions & 5 deletions wdl/src/main/scala/wdl/expression/TypeEvaluator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,13 @@ case class TypeEvaluator(override val lookup: String => WomType, override val fu
evaluate(a.getAttribute("lhs")).flatMap {
case o: WdlCallOutputsObjectType =>
o.call.outputs.find(_.unqualifiedName == rhs.getSourceString) match {
case Some(taskOutput) =>
from map { source =>
evaluate(taskOutput.requiredExpression.ast) map { t => DeclarationInterface.relativeWdlType(source, taskOutput, t) }
} getOrElse evaluate(taskOutput.requiredExpression.ast)
case Some(taskOutput) =>
val t = taskOutput.womType
val relative = from match {
case None => t
case Some(scope) => DeclarationInterface.relativeWdlType(scope, taskOutput, t)
}
Success(relative)
case None => Failure(new WomExpressionException(s"Could not find key ${rhs.getSourceString}"))
}
case WomPairType(leftType, rightType) =>
Expand Down Expand Up @@ -132,4 +135,3 @@ case class TypeEvaluator(override val lookup: String => WomType, override val fu
}
}
}

173 changes: 173 additions & 0 deletions wdl/src/test/scala/wdl/expression/DNAxTypeEvalTest.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
package wdl.expression

import org.scalatest.{FlatSpec, Matchers}
import scala.util.{Success, Try}
import wdl.{Scope, Scatter, WdlCall, WdlExpression, WdlNamespace, WdlNamespaceWithWorkflow}
import wom.types._

class DNAxTypeEvalTest extends FlatSpec with Matchers {

val wdlCode =
"""|task Add {
| Int a
| Int b
|
| command {
| }
| output {
| Int result = a + b
| }
|}
|
|workflow dict {
| Map[Int, Int] mII = {1: 10, 2: 11}
| Map[Int, Float] mIF = {1: 1.2, 10: 113.0}
|
| scatter(pair in mII) {
| Int valueII = pair.right
| }
|
| scatter(pair in mIF) {
| call Add {
| input: a=pair.left, b=5
| }
| }
|
| output {
| valueII
| }
|}
|""".stripMargin


// Figure out the type of an expression
def evalType(expr: WdlExpression, parent: Scope) : Try[WomType] = {
expr.evaluateType(WdlNamespace.lookupType(parent),
new WdlStandardLibraryFunctionsType,
Some(parent))
}

it should "correctly evaluate expression types" in {
val ns = WdlNamespaceWithWorkflow.load(wdlCode, Seq.empty).get
val wf = ns.workflow

val call:WdlCall = wf.findCallByName("Add") match {
case None => throw new Exception(s"Call Add not found in WDL file")
case Some(call) => call
}
val ssc:Scatter = wf.scatters.head

call.inputMappings.foreach { case (_, expr) =>
val t:Try[WomType] = evalType(expr, ssc)
t should equal(Success(WomIntegerType))
}
}


val wdlCode2 =
"""|task Copy {
| File src
| String basename
|
| command <<<
| cp ${src} ${basename}.txt
| sort ${src} > ${basename}.sorted.txt
| >>>
| output {
| File outf = "${basename}.txt"
| File outf_sorted = "${basename}.sorted.txt"
| }
|}
|
|
|workflow files {
| File f
|
| call Copy { input : src=f, basename="tearFrog" }
| call Copy as Copy2 { input : src=Copy.outf, basename="mixing" }
|
| output {
| Copy2.outf_sorted
| }
|}
|""".stripMargin

it should "correctly evaluate Strings and File types" in {
val ns = WdlNamespaceWithWorkflow.load(wdlCode2, Seq.empty).get
val wf = ns.workflow

val copy2call:WdlCall = wf.findCallByName("Copy2") match {
case None => throw new Exception(s"Call Add not found in WDL file")
case Some(call) => call
}

val e1 = copy2call.inputMappings("src")
val t1 = evalType(e1, copy2call)
t1 should equal(Success(WomFileType))

val e2 = copy2call.inputMappings("basename")
val t2 = evalType(e2, copy2call)
t2 should equal(Success(WomStringType))
}


val wdlCode3 =
"""|
|task Inc {
| Int i
|
| command <<<
| python -c "print(${i} + 1)"
| >>>
| output {
| Int result = read_int(stdout())
| }
|}
|
|task Mod7 {
| Int i
|
| command <<<
| python -c "print(${i} % 7)"
| >>>
| output {
| Int result = read_int(stdout())
| }
|}
|
|workflow math {
| Int n = 5
|
| scatter (k in range(length(n))) {
| call Mod7 {input: i=k}
| }
|
| scatter (k in Mod7.result) {
| call Inc {input: i=k}
| }
|
| output {
| Inc.result
| }
|}
|""".stripMargin

it should "take context into account" in {
val ns = WdlNamespaceWithWorkflow.load(wdlCode3, Seq.empty).get
val wf = ns.workflow

val incCall:WdlCall = wf.findCallByName("Inc") match {
case None => throw new Exception(s"Call Add not found in WDL file")
case Some(call) => call
}

val e1 = incCall.inputMappings("i")
val t1 = evalType(e1, incCall)
t1 should equal(Success(WomIntegerType))

val output = wf.outputs.head
val e2 = output.requiredExpression
val t2 = evalType(e2, wf)
t2 should equal(Success(WomMaybeEmptyArrayType(WomIntegerType)))
}
}
6 changes: 3 additions & 3 deletions wdl/src/test/scala/wom/WdlSubworkflowWomSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class WdlSubworkflowWomSpec extends FlatSpec with Matchers {
| Int x
| call import_me.inner as inner { input: i = x }
| output {
| Array[String] out = inner.out
| Array[String] out = [inner.out]
| }
|}""".stripMargin

Expand Down Expand Up @@ -51,7 +51,7 @@ class WdlSubworkflowWomSpec extends FlatSpec with Matchers {
workflowSource = outerWdl,
resource = None,
importResolver = Some(Seq(innerResolver))).get.asInstanceOf[WdlNamespaceWithWorkflow]

val outerWorkflowGraph = namespace.workflow.womDefinition.map(_.graph)

outerWorkflowGraph match {
Expand Down Expand Up @@ -165,7 +165,7 @@ class WdlSubworkflowWomSpec extends FlatSpec with Matchers {
workflowGraph.outputNodes.foreach(_.upstream should be(Set(scatter)))
}
}

it should "support conversion of sub workflows with identical names" in {
val outerWdl =
"""import "import_me.wdl" as import_me
Expand Down

0 comments on commit 6b203f9

Please sign in to comment.