From 80559d8db1ba9a314c6189d7c010e54573cd4683 Mon Sep 17 00:00:00 2001 From: Ohad Rodeh Date: Tue, 16 Jul 2019 05:57:27 -0700 Subject: [PATCH] Nested scatters [BA-5803] (#5061) * Adding a flag to the configuration file; do we want to distinguish between inner and outer scatters? * Using the innerOuterScatter flag in the WOM transforms code. Create a sub-workflow from the inner scatter, only if the flag is turned on. * Fixing code review comments * Fixing code review comments * Changing womParse to wom-parse * Adding test for the WomParseConfig convert-nested-scatter-to-subworkflow flag * Passing the flag more directly, without using the configuration. --- .../cwl/CwlV1_0LanguageFactory.scala | 6 +- .../cromwell/languages/LanguageFactory.scala | 3 +- .../biscayne/WdlBiscayneLanguageFactory.scala | 8 ++- .../wdl/draft2/WdlDraft2LanguageFactory.scala | 6 +- .../wdl/draft3/WdlDraft3LanguageFactory.scala | 8 ++- .../src/test/cases/two_level_scatter.wdl | 12 ++++ .../ast2wdlom/WdlFileToWdlomSpec.scala | 35 ++++++++++ .../wdlom2wom/WdlFileToWomSpec.scala | 66 ++++++++++++++++++- .../wdlom2wom/FileElementToWomBundle.scala | 6 +- ...nitionElementToWomWorkflowDefinition.scala | 16 +++-- .../graph/IfElementToGraphNode.scala | 6 +- .../graph/ScatterElementToGraphNode.scala | 21 ++++-- .../WorkflowGraphElementToGraphNode.scala | 5 +- wom/src/main/resources/reference.conf | 7 ++ .../main/scala/womtool/graph/WomGraph.scala | 4 +- 15 files changed, 187 insertions(+), 22 deletions(-) create mode 100644 wdl/transforms/draft3/src/test/cases/two_level_scatter.wdl diff --git a/languageFactories/cwl-v1-0/src/main/scala/languages/cwl/CwlV1_0LanguageFactory.scala b/languageFactories/cwl-v1-0/src/main/scala/languages/cwl/CwlV1_0LanguageFactory.scala index c138f96155d..dcfa5963464 100644 --- a/languageFactories/cwl-v1-0/src/main/scala/languages/cwl/CwlV1_0LanguageFactory.scala +++ b/languageFactories/cwl-v1-0/src/main/scala/languages/cwl/CwlV1_0LanguageFactory.scala @@ -54,7 +54,11 @@ class CwlV1_0LanguageFactory(override val config: Config) extends LanguageFactor } yield validatedWomNamespace } - override def getWomBundle(workflowSource: WorkflowSource, workflowOptionsJson: WorkflowOptionsJson, importResolvers: List[ImportResolver], languageFactories: List[LanguageFactory]): Checked[WomBundle] = + override def getWomBundle(workflowSource: WorkflowSource, + workflowOptionsJson: WorkflowOptionsJson, + importResolvers: List[ImportResolver], + languageFactories: List[LanguageFactory], + convertNestedScatterToSubworkflow : Boolean = true): Checked[WomBundle] = enabledCheck flatMap { _ => "No getWomBundle method implemented in CWL v1".invalidNelCheck } override def createExecutable(womBundle: WomBundle, inputs: WorkflowJson, ioFunctions: IoFunctionSet): Checked[ValidatedWomNamespace] = diff --git a/languageFactories/language-factory-core/src/main/scala/cromwell/languages/LanguageFactory.scala b/languageFactories/language-factory-core/src/main/scala/cromwell/languages/LanguageFactory.scala index 1cea1435ea8..b5046b4ea8e 100644 --- a/languageFactories/language-factory-core/src/main/scala/cromwell/languages/LanguageFactory.scala +++ b/languageFactories/language-factory-core/src/main/scala/cromwell/languages/LanguageFactory.scala @@ -36,7 +36,8 @@ trait LanguageFactory { def getWomBundle(workflowSource: WorkflowSource, workflowOptionsJson: WorkflowOptionsJson, importResolvers: List[ImportResolver], - languageFactories: List[LanguageFactory]): Checked[WomBundle] + languageFactories: List[LanguageFactory], + convertNestedScatterToSubworkflow : Boolean = true): Checked[WomBundle] def createExecutable(womBundle: WomBundle, inputs: WorkflowJson, diff --git a/languageFactories/wdl-biscayne/src/main/scala/languages/wdl/biscayne/WdlBiscayneLanguageFactory.scala b/languageFactories/wdl-biscayne/src/main/scala/languages/wdl/biscayne/WdlBiscayneLanguageFactory.scala index 1ee1b5b004a..916ee335330 100644 --- a/languageFactories/wdl-biscayne/src/main/scala/languages/wdl/biscayne/WdlBiscayneLanguageFactory.scala +++ b/languageFactories/wdl-biscayne/src/main/scala/languages/wdl/biscayne/WdlBiscayneLanguageFactory.scala @@ -46,9 +46,13 @@ class WdlBiscayneLanguageFactory(override val config: Config) extends LanguageFa } - override def getWomBundle(workflowSource: WorkflowSource, workflowOptionsJson: WorkflowOptionsJson, importResolvers: List[ImportResolver], languageFactories: List[LanguageFactory]): Checked[WomBundle] = { + override def getWomBundle(workflowSource: WorkflowSource, + workflowOptionsJson: WorkflowOptionsJson, + importResolvers: List[ImportResolver], + languageFactories: List[LanguageFactory], + convertNestedScatterToSubworkflow : Boolean = true): Checked[WomBundle] = { val checkEnabled: CheckedAtoB[FileStringParserInput, FileStringParserInput] = CheckedAtoB.fromCheck(x => enabledCheck map(_ => x)) - val converter: CheckedAtoB[FileStringParserInput, WomBundle] = checkEnabled andThen stringToAst andThen wrapAst andThen astToFileElement.map(FileElementToWomBundleInputs(_, workflowOptionsJson, importResolvers, languageFactories, workflowDefinitionElementToWomWorkflowDefinition, taskDefinitionElementToWomTaskDefinition)) andThen fileElementToWomBundle + val converter: CheckedAtoB[FileStringParserInput, WomBundle] = checkEnabled andThen stringToAst andThen wrapAst andThen astToFileElement.map(FileElementToWomBundleInputs(_, workflowOptionsJson, convertNestedScatterToSubworkflow, importResolvers, languageFactories, workflowDefinitionElementToWomWorkflowDefinition, taskDefinitionElementToWomTaskDefinition)) andThen fileElementToWomBundle converter.run(FileStringParserInput(workflowSource, "input.wdl")) } diff --git a/languageFactories/wdl-draft2/src/main/scala/languages/wdl/draft2/WdlDraft2LanguageFactory.scala b/languageFactories/wdl-draft2/src/main/scala/languages/wdl/draft2/WdlDraft2LanguageFactory.scala index 07ca2744fd9..4bb86fc9ac9 100644 --- a/languageFactories/wdl-draft2/src/main/scala/languages/wdl/draft2/WdlDraft2LanguageFactory.scala +++ b/languageFactories/wdl-draft2/src/main/scala/languages/wdl/draft2/WdlDraft2LanguageFactory.scala @@ -113,7 +113,11 @@ class WdlDraft2LanguageFactory(override val config: Config) extends LanguageFact } } - override def getWomBundle(workflowSource: WorkflowSource, workflowOptionsJson: WorkflowOptionsJson, importResolvers: List[ImportResolver], languageFactories: List[LanguageFactory]): Checked[WomBundle] = { + override def getWomBundle(workflowSource: WorkflowSource, + workflowOptionsJson: WorkflowOptionsJson, + importResolvers: List[ImportResolver], + languageFactories: List[LanguageFactory], + convertNestedScatterToSubworkflow : Boolean = true): Checked[WomBundle] = { for { _ <- enabledCheck namespace <- WdlNamespace.loadUsingSource(workflowSource, None, Some(importResolvers map resolverConverter)).toChecked diff --git a/languageFactories/wdl-draft3/src/main/scala/languages/wdl/draft3/WdlDraft3LanguageFactory.scala b/languageFactories/wdl-draft3/src/main/scala/languages/wdl/draft3/WdlDraft3LanguageFactory.scala index d4d3609b3be..ca907f6c52d 100644 --- a/languageFactories/wdl-draft3/src/main/scala/languages/wdl/draft3/WdlDraft3LanguageFactory.scala +++ b/languageFactories/wdl-draft3/src/main/scala/languages/wdl/draft3/WdlDraft3LanguageFactory.scala @@ -47,9 +47,13 @@ class WdlDraft3LanguageFactory(override val config: Config) extends LanguageFact } - override def getWomBundle(workflowSource: WorkflowSource, workflowOptionsJson: WorkflowOptionsJson, importResolvers: List[ImportResolver], languageFactories: List[LanguageFactory]): Checked[WomBundle] = { + override def getWomBundle(workflowSource: WorkflowSource, + workflowOptionsJson: WorkflowOptionsJson, + importResolvers: List[ImportResolver], + languageFactories: List[LanguageFactory], + convertNestedScatterToSubworkflow : Boolean = true): Checked[WomBundle] = { val checkEnabled: CheckedAtoB[FileStringParserInput, FileStringParserInput] = CheckedAtoB.fromCheck(x => enabledCheck map(_ => x)) - val converter: CheckedAtoB[FileStringParserInput, WomBundle] = checkEnabled andThen stringToAst andThen wrapAst andThen astToFileElement.map(FileElementToWomBundleInputs(_, workflowOptionsJson, importResolvers, languageFactories, workflowDefinitionElementToWomWorkflowDefinition, taskDefinitionElementToWomTaskDefinition)) andThen fileElementToWomBundle + val converter: CheckedAtoB[FileStringParserInput, WomBundle] = checkEnabled andThen stringToAst andThen wrapAst andThen astToFileElement.map(FileElementToWomBundleInputs(_, workflowOptionsJson, convertNestedScatterToSubworkflow, importResolvers, languageFactories, workflowDefinitionElementToWomWorkflowDefinition, taskDefinitionElementToWomTaskDefinition)) andThen fileElementToWomBundle converter.run(FileStringParserInput(workflowSource, "input.wdl")) } diff --git a/wdl/transforms/draft3/src/test/cases/two_level_scatter.wdl b/wdl/transforms/draft3/src/test/cases/two_level_scatter.wdl new file mode 100644 index 00000000000..44ff02141e4 --- /dev/null +++ b/wdl/transforms/draft3/src/test/cases/two_level_scatter.wdl @@ -0,0 +1,12 @@ +version 1.0 + +workflow two_level_scatter { + + Array[Int] indices = [1,2,3] + + scatter(a in indices) { + scatter(b in indices) { + Int x = a + b + } + } +} diff --git a/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/ast2wdlom/WdlFileToWdlomSpec.scala b/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/ast2wdlom/WdlFileToWdlomSpec.scala index b586c776234..56961f9d747 100644 --- a/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/ast2wdlom/WdlFileToWdlomSpec.scala +++ b/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/ast2wdlom/WdlFileToWdlomSpec.scala @@ -581,6 +581,41 @@ object WdlFileToWdlomSpec { ), Vector() ), + "two_level_scatter" -> FileElement( + Vector(), + Vector(), + Vector(WorkflowDefinitionElement( + "two_level_scatter", + None, + Set( + IntermediateValueDeclarationElement( + ArrayTypeElement(PrimitiveTypeElement(WomIntegerType)), + "indices", + ArrayLiteral(Vector(PrimitiveLiteralExpressionElement(WomInteger(1)), PrimitiveLiteralExpressionElement(WomInteger(2)), PrimitiveLiteralExpressionElement(WomInteger(3)))) + ), + ScatterElement( + scatterName = "ScatterAt8_11", + IdentifierLookup("indices"), + "a", + Vector( + ScatterElement( + scatterName = "ScatterAt9_13", + IdentifierLookup("indices"), "b", + Vector( + IntermediateValueDeclarationElement(PrimitiveTypeElement(WomIntegerType), "x", Add(IdentifierLookup("a"), IdentifierLookup("b")))), + sourceLocation = None + ) + ), + None + ) + ), + None, + None, + None, + Some(SourceFileLocation(3))) + ), + Vector() + ), "simple_conditional" -> FileElement( imports = Vector.empty, structs = Vector.empty, diff --git a/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/wdlom2wom/WdlFileToWomSpec.scala b/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/wdlom2wom/WdlFileToWomSpec.scala index 91093815186..3a3b1ebf1a5 100644 --- a/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/wdlom2wom/WdlFileToWomSpec.scala +++ b/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/wdlom2wom/WdlFileToWomSpec.scala @@ -16,6 +16,7 @@ import wom.callable.MetaValueElement._ import wom.callable.{CallableTaskDefinition, WorkflowDefinition} import wom.executable.WomBundle import wom.graph.expression.{ExposedExpressionNode, TaskCallInputExpressionNode} +import wom.graph.{ScatterNode, WorkflowCallNode} import wom.types._ class WdlFileToWomSpec extends FlatSpec with Matchers { @@ -41,7 +42,7 @@ class WdlFileToWomSpec extends FlatSpec with Matchers { } testOrIgnore { - val converter: CheckedAtoB[File, WomBundle] = fileToAst andThen wrapAst andThen astToFileElement.map(fe => FileElementToWomBundleInputs(fe, "{}", List.empty, List.empty, workflowDefinitionElementToWomWorkflowDefinition, taskDefinitionElementToWomTaskDefinition)) andThen fileElementToWomBundle + val converter: CheckedAtoB[File, WomBundle] = fileToAst andThen wrapAst andThen astToFileElement.map(fe => FileElementToWomBundleInputs(fe, "{}", convertNestedScatterToSubworkflow = true, List.empty, List.empty, workflowDefinitionElementToWomWorkflowDefinition, taskDefinitionElementToWomTaskDefinition)) andThen fileElementToWomBundle converter.run(testCase) match { case Right(bundle) => validators(testName).apply(bundle) @@ -52,6 +53,68 @@ class WdlFileToWomSpec extends FlatSpec with Matchers { } } + // There is a scatter within a scatter + // + // scatter(a in indices) { + // scatter(b in indices) { + // Int x = a + b + // } + // } + // + it should "be able to leave nested scatters intact" in { + val converter: CheckedAtoB[File, WomBundle] = fileToAst andThen wrapAst andThen astToFileElement.map(fe => FileElementToWomBundleInputs(fe, "{}", convertNestedScatterToSubworkflow = false, List.empty, List.empty, workflowDefinitionElementToWomWorkflowDefinition, taskDefinitionElementToWomTaskDefinition)) andThen fileElementToWomBundle + + val twoLevelScatterFile = File("wdl/transforms/draft3/src/test/cases/two_level_scatter.wdl") + + converter.run(twoLevelScatterFile) match { + case Right(bundle) => + val wf = bundle.primaryCallable.get.asInstanceOf[WorkflowDefinition] + val graph = wf.innerGraph + + // get the top scatter node + graph.scatters.size shouldBe(1) + val topScatter : ScatterNode = graph.scatters.toVector.head + val wfCalls = graph.allNodes.filterByType[WorkflowCallNode] + + // don't generate any sub-workflows + wfCalls.size shouldBe(0) + + // there should be one scatter inside the top scatter + val innerGraph = topScatter.innerGraph + innerGraph.scatters.size shouldBe(1) + Succeeded + + case Left(errors) => + val formattedErrors = errors.toList.mkString(System.lineSeparator(), System.lineSeparator(), System.lineSeparator()) + fail(s"Failed to create WOM bundle: $formattedErrors") + } + } + + + it should "split a nested scatter into a toplevel scatter, and a bottom sub-workflow" in { + val converter: CheckedAtoB[File, WomBundle] = fileToAst andThen wrapAst andThen astToFileElement.map(fe => FileElementToWomBundleInputs(fe, "{}", convertNestedScatterToSubworkflow = true, List.empty, List.empty, workflowDefinitionElementToWomWorkflowDefinition, taskDefinitionElementToWomTaskDefinition)) andThen fileElementToWomBundle + + val twoLevelScatterFile = File("wdl/transforms/draft3/src/test/cases/two_level_scatter.wdl") + + converter.run(twoLevelScatterFile) match { + case Right(bundle) => + val wf = bundle.primaryCallable.get.asInstanceOf[WorkflowDefinition] + val graph = wf.innerGraph + + // There should be just one scatter. + graph.scatters.size shouldBe(1) + val wfCalls = graph.allNodes.filterByType[WorkflowCallNode] + + // There should be a call to a generated sub-workflow in the graph + wfCalls.size shouldBe(1) + Succeeded + case Left(errors) => + val formattedErrors = errors.toList.mkString(System.lineSeparator(), System.lineSeparator(), System.lineSeparator()) + fail(s"Failed to create WOM bundle: $formattedErrors") + } + } + + private val validators: Map[String, WomBundle => Assertion] = Map( "declaration_chain" -> anyWomWillDo, "empty_workflow" -> anyWomWillDo, @@ -68,6 +131,7 @@ class WdlFileToWomSpec extends FlatSpec with Matchers { "simple_scatter" -> anyWomWillDo, "ogin_scatter" -> anyWomWillDo, "nested_scatter" -> anyWomWillDo, + "two_level_scatter" -> anyWomWillDo, "simple_conditional" -> anyWomWillDo, "lots_of_nesting" -> anyWomWillDo, "standalone_task" -> anyWomWillDo, diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/FileElementToWomBundle.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/FileElementToWomBundle.scala index 4b183dde7a8..39a8ab79a72 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/FileElementToWomBundle.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/FileElementToWomBundle.scala @@ -46,7 +46,10 @@ object FileElementToWomBundle { val workflowsValidation: ErrorOr[Vector[WorkflowDefinition]] = { a.fileElement.workflows.toVector.traverse { workflowDefinition => - val convertInputs = WorkflowDefinitionConvertInputs(workflowDefinition, allStructs, localTaskMapping ++ imports.flatMap(_.allCallables)) + val convertInputs = WorkflowDefinitionConvertInputs(workflowDefinition, + allStructs, + localTaskMapping ++ imports.flatMap(_.allCallables), + a.convertNestedScatterToSubworkflow) a.workflowConverter.run(convertInputs).toValidated } } @@ -129,6 +132,7 @@ object FileElementToWomBundle { final case class FileElementToWomBundleInputs(fileElement: FileElement, workflowOptionsJson: WorkflowOptionsJson, + convertNestedScatterToSubworkflow : Boolean, importResolvers: List[ImportResolver], languageFactories: List[LanguageFactory], workflowConverter: CheckedAtoB[WorkflowDefinitionConvertInputs, WorkflowDefinition], diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/WorkflowDefinitionElementToWomWorkflowDefinition.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/WorkflowDefinitionElementToWomWorkflowDefinition.scala index 6162d627b98..f41a17c5883 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/WorkflowDefinitionElementToWomWorkflowDefinition.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/WorkflowDefinitionElementToWomWorkflowDefinition.scala @@ -20,7 +20,10 @@ import wdl.transforms.base.wdlom2wom.graph.renaming._ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util { - final case class WorkflowDefinitionConvertInputs(definitionElement: WorkflowDefinitionElement, typeAliases: Map[String, WomType], callables: Map[String, Callable]) + final case class WorkflowDefinitionConvertInputs(definitionElement: WorkflowDefinitionElement, + typeAliases: Map[String, WomType], + callables: Map[String, Callable], + convertNestedScatterToSubworkflow : Boolean) def convert(b: WorkflowDefinitionConvertInputs) (implicit expressionValueConsumer: ExpressionValueConsumer[ExpressionElement], @@ -39,7 +42,10 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util { a.definitionElement.inputsSection.toSeq.flatMap(_.inputDeclarations) ++ a.definitionElement.outputsSection.toSeq.flatMap(_.outputs) - val innerGraph: ErrorOr[WomGraph] = convertGraphElements(GraphLikeConvertInputs(graphNodeElements, Set.empty, Map.empty, a.typeAliases, a.definitionElement.name, insideAScatter = false, a.callables)) + val innerGraph: ErrorOr[WomGraph] = convertGraphElements(GraphLikeConvertInputs(graphNodeElements, Set.empty, Map.empty, a.typeAliases, a.definitionElement.name, + insideAScatter = false, + convertNestedScatterToSubworkflow = b.convertNestedScatterToSubworkflow, + a.callables)) // NB: isEmpty means "not isDefined". We specifically do NOT add defaults if the output section is defined but empty. val withDefaultOutputs: ErrorOr[WomGraph] = if (a.definitionElement.outputsSection.isEmpty) { innerGraph map { WomGraphMakerTools.addDefaultOutputs(_, Some(WomIdentifier(a.definitionElement.name))) } @@ -60,6 +66,7 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util { typeAliases: Map[String, WomType], workflowName: String, insideAScatter: Boolean, + convertNestedScatterToSubworkflow: Boolean, callables: Map[String, Callable]) def convertGraphElements(a: GraphLikeConvertInputs) @@ -77,7 +84,7 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util { for { linkedGraph <- LinkedGraphMaker.make(nodes = a.graphElements, seedGeneratedValueHandles ++ finished, typeAliases = a.typeAliases, callables = a.callables) - womGraph <- makeWomGraph(linkedGraph, a.seedNodes, a.externalUpstreamCalls, a.workflowName, a.insideAScatter, a.callables) + womGraph <- makeWomGraph(linkedGraph, a.seedNodes, a.externalUpstreamCalls, a.workflowName, a.insideAScatter, a.convertNestedScatterToSubworkflow, a.callables) } yield womGraph } @@ -86,6 +93,7 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util { externalUpstreamCalls: Map[String, CallNode], workflowName: String, insideAScatter: Boolean, + convertNestedScatterToSubworkflow : Boolean, callables: Map[String, Callable]) (implicit expressionValueConsumer: ExpressionValueConsumer[ExpressionElement], fileEvaluator: FileEvaluator[ExpressionElement], @@ -111,7 +119,7 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util { val generatedGraphNodesValidation: ErrorOr[Set[GraphNode]] = WorkflowGraphElementToGraphNode.convert( - GraphNodeMakerInputs(next, upstreamCallNodes, linkedGraph.consumedValueLookup, availableValues, linkedGraph.typeAliases, workflowName, insideAScatter, callables)) + GraphNodeMakerInputs(next, upstreamCallNodes, linkedGraph.consumedValueLookup, availableValues, linkedGraph.typeAliases, workflowName, insideAScatter, convertNestedScatterToSubworkflow, callables)) generatedGraphNodesValidation map { nextGraphNodes: Set[GraphNode] => currentList ++ nextGraphNodes } } } diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/IfElementToGraphNode.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/IfElementToGraphNode.scala index 31f2f4b3e51..230dec2642d 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/IfElementToGraphNode.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/IfElementToGraphNode.scala @@ -73,7 +73,10 @@ object IfElementToGraphNode { OuterGraphInputNode(WomIdentifier(name), port, preserveScatterIndex = true) }).toSet - val graphLikeConvertInputs = GraphLikeConvertInputs(graphElements.toSet, ogins, foundOuterGenerators.completionPorts, a.availableTypeAliases, a.workflowName, insideAScatter = a.insideAnotherScatter, a.callables) + val graphLikeConvertInputs = GraphLikeConvertInputs(graphElements.toSet, ogins, foundOuterGenerators.completionPorts, a.availableTypeAliases, a.workflowName, + insideAScatter = a.insideAnotherScatter, + convertNestedScatterToSubworkflow = a.convertNestedScatterToSubworkflow, + a.callables) val innerGraph: ErrorOr[Graph] = WorkflowDefinitionElementToWomWorkflowDefinition.convertGraphElements(graphLikeConvertInputs) innerGraph map { ig => @@ -92,4 +95,5 @@ final case class ConditionalNodeMakerInputs(node: IfElement, availableTypeAliases: Map[String, WomType], workflowName: String, insideAnotherScatter: Boolean, + convertNestedScatterToSubworkflow : Boolean, callables: Map[String, Callable]) diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/ScatterElementToGraphNode.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/ScatterElementToGraphNode.scala index 9d810035ad8..9618c1fd1b5 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/ScatterElementToGraphNode.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/ScatterElementToGraphNode.scala @@ -32,9 +32,15 @@ object ScatterElementToGraphNode { fileEvaluator: FileEvaluator[ExpressionElement], typeEvaluator: TypeEvaluator[ExpressionElement], valueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[Set[GraphNode]] = - if (a.insideAnotherScatter) { - convertInnerScatter(a) + if (a.convertNestedScatterToSubworkflow) { + // Create a sub-workflow from the inner scatter. + if (a.insideAnotherScatter) { + convertInnerScatter(a) + } else { + convertOuterScatter(a) + } } else { + // do not do anything special with inner scatters. convertOuterScatter(a) } @@ -94,7 +100,10 @@ object ScatterElementToGraphNode { OuterGraphInputNode(WomIdentifier(name), port, preserveScatterIndex = false) }).toSet - val graphLikeConvertInputs = GraphLikeConvertInputs(graphElements.toSet, ogins ++ Set(womInnerGraphScatterVariableInput), foundOuterGenerators.completionPorts, a.availableTypeAliases, a.workflowName, insideAScatter = true, a.callables) + val graphLikeConvertInputs = GraphLikeConvertInputs(graphElements.toSet, ogins ++ Set(womInnerGraphScatterVariableInput), foundOuterGenerators.completionPorts, a.availableTypeAliases, a.workflowName, + insideAScatter = true, + convertNestedScatterToSubworkflow = a.convertNestedScatterToSubworkflow, + a.callables) val innerGraph: ErrorOr[Graph] = WorkflowDefinitionElementToWomWorkflowDefinition.convertGraphElements(graphLikeConvertInputs) innerGraph map { ig => @@ -119,7 +128,10 @@ object ScatterElementToGraphNode { } val subWorkflowGraphValidation: ErrorOr[Graph] = subWorkflowInputsValidation flatMap { subWorkflowInputs => - val graphLikeConvertInputs = GraphLikeConvertInputs(Set(a.node), subWorkflowInputs, Map.empty, a.availableTypeAliases, a.workflowName, insideAScatter = false, a.callables) + val graphLikeConvertInputs = GraphLikeConvertInputs(Set(a.node), subWorkflowInputs, Map.empty, a.availableTypeAliases, a.workflowName, + insideAScatter = false, + convertNestedScatterToSubworkflow = a.convertNestedScatterToSubworkflow, + a.callables) val subWorkflowGraph = WorkflowDefinitionElementToWomWorkflowDefinition.convertGraphElements(graphLikeConvertInputs) subWorkflowGraph map { WomGraphMakerTools.addDefaultOutputs(_) } } @@ -174,4 +186,5 @@ final case class ScatterNodeMakerInputs(node: ScatterElement, availableTypeAliases: Map[String, WomType], workflowName: String, insideAnotherScatter: Boolean, + convertNestedScatterToSubworkflow: Boolean, callables: Map[String, Callable]) diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/WorkflowGraphElementToGraphNode.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/WorkflowGraphElementToGraphNode.scala index 8c93d4136c4..7c6ea313264 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/WorkflowGraphElementToGraphNode.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/WorkflowGraphElementToGraphNode.scala @@ -50,11 +50,11 @@ object WorkflowGraphElementToGraphNode { result.contextualizeErrors(s"process declaration '${typeElement.toWdlV1} $name = ${expr.toWdlV1}'") case se: ScatterElement => - val scatterMakerInputs = ScatterNodeMakerInputs(se, a.upstreamCalls, a.linkableValues, a.linkablePorts, a.availableTypeAliases, a.workflowName, a.insideAScatter, a.callables) + val scatterMakerInputs = ScatterNodeMakerInputs(se, a.upstreamCalls, a.linkableValues, a.linkablePorts, a.availableTypeAliases, a.workflowName, a.insideAScatter, a.convertNestedScatterToSubworkflow, a.callables) ScatterElementToGraphNode.convert(scatterMakerInputs) case ie: IfElement => - val ifMakerInputs = ConditionalNodeMakerInputs(ie, a.upstreamCalls, a.linkableValues, a.linkablePorts, a.availableTypeAliases, a.workflowName, a.insideAScatter, a.callables) + val ifMakerInputs = ConditionalNodeMakerInputs(ie, a.upstreamCalls, a.linkableValues, a.linkablePorts, a.availableTypeAliases, a.workflowName, a.insideAScatter, a.convertNestedScatterToSubworkflow, a.callables) IfElementToGraphNode.convert(ifMakerInputs) case ce: CallElement => @@ -80,4 +80,5 @@ final case class GraphNodeMakerInputs(node: WorkflowGraphElement, availableTypeAliases: Map[String, WomType], workflowName: String, insideAScatter: Boolean, + convertNestedScatterToSubworkflow : Boolean, callables: Map[String, Callable]) diff --git a/wom/src/main/resources/reference.conf b/wom/src/main/resources/reference.conf index 6073365bc31..1907a23eac6 100644 --- a/wom/src/main/resources/reference.conf +++ b/wom/src/main/resources/reference.conf @@ -4,3 +4,10 @@ yaml { # The maximum nested depth for nodes (sequences + mappings) that will be parsed in a YAML max-depth = 1000 } + +wom-parse { + # Assume there is WDL code that has a nested scatter. Should we distinguish between + # the inner and outer loops? The default is yes. Different code is created for the inner + # loop vs. the outer loop. + convert-nested-scatter-to-subworkflow = true +} diff --git a/womtool/src/main/scala/womtool/graph/WomGraph.scala b/womtool/src/main/scala/womtool/graph/WomGraph.scala index d30f4b024e1..02c60de376c 100644 --- a/womtool/src/main/scala/womtool/graph/WomGraph.scala +++ b/womtool/src/main/scala/womtool/graph/WomGraph.scala @@ -160,7 +160,7 @@ class WomGraph(graphName: String, graph: Graph) { } object WomGraph { - + implicit val cwlPreProcessor = CwlPreProcessor.noLogging final case class WorkflowDigraph(workflowName: String, digraph: NodesAndLinks) @@ -188,7 +188,7 @@ object WomGraph { firstLine.startsWith("version 1.0") } val womBundle: Checked[WomBundle] = if (version1) { - val converter: CheckedAtoB[File, WomBundle] = fileToAst andThen wrapAst andThen astToFileElement.map(FileElementToWomBundleInputs(_, "{}", List.empty, List.empty, workflowDefinitionElementToWomWorkflowDefinition, taskDefinitionElementToWomTaskDefinition)) andThen fileElementToWomBundle + val converter: CheckedAtoB[File, WomBundle] = fileToAst andThen wrapAst andThen astToFileElement.map(FileElementToWomBundleInputs(_, "{}", convertNestedScatterToSubworkflow = true, List.empty, List.empty, workflowDefinitionElementToWomWorkflowDefinition, taskDefinitionElementToWomTaskDefinition)) andThen fileElementToWomBundle converter.run(File(filePath)) } else {