daphne-eu / daphne

DAPHNE: An Open and Extensible System Infrastructure for Integrated Data Analysis Pipelines
Apache License 2.0
67 stars 62 forks source link

[MINOR] Parsing IR fix. #634

Closed aristotelis96 closed 1 year ago

aristotelis96 commented 1 year ago

While parsing a matrix object the "sparsity" and "rerpesentation" arguments can both exist. In that case trying to parse one of them while multiple exist, can make the parser emmit errors. We should consider them optional while trying to parse them in the loop.

This is currently an issue for the distributed runtime, since distributed workers emmit errors while parsing the IR sent by the coordinator.

Example IR that a matrix contains both "sparsity" and "representation" and parsing it emits errors:

func.func @dist(%arg0: !daphne.Matrix<?x403394xf64:sp[2.0816421641779212E-5]:rep[sparse]>, %arg1: !daphne.Matrix<?x?xf64>, %arg2: !daphne.Matrix<?x1xf64>) -> (!daphne.Matrix<?x?xf64>, !daphne.Matrix<?x?xf64>) {
  %0 = "daphne.ewMul"(%arg0, %arg1) : (!daphne.Matrix<?x403394xf64:sp[2.0816421641779212E-5]:rep[sparse]>, !daphne.Matrix<?x?xf64>) -> !daphne.Matrix<?x?xf64:sp[2.0816421641779212E-5]:rep[sparse]>
  %1 = "daphne.maxRow"(%0) : (!daphne.Matrix<?x?xf64:sp[2.0816421641779212E-5]:rep[sparse]>) -> !daphne.Matrix<?x?xf64>
  %2 = "daphne.ewMax"(%1, %arg2) : (!daphne.Matrix<?x?xf64>, !daphne.Matrix<?x1xf64>) -> !daphne.Matrix<?x?xf64>
  %3 = "daphne.ewNeq"(%2, %arg2) : (!daphne.Matrix<?x?xf64>, !daphne.Matrix<?x1xf64>) -> !daphne.Matrix<?x?xf64>
  "daphne.return"(%2, %3) : (!daphne.Matrix<?x?xf64>, !daphne.Matrix<?x?xf64>) -> ()
}
aristotelis96 commented 1 year ago

@pdamme @corepointer I would kindly ask for a confirmation from your side that this is the correct way to parse the dialect before merging this.

pdamme commented 1 year ago

By the way, this PR is related to #154 and #155.

aristotelis96 commented 1 year ago

Thank you @pdamme for your feedback. I've slightly extended the parser test in order to check IR-parsing after MatrixRepresentationPass, since these changes affect IR produced by that pass.