Baunsgaard commented on code in PR #1672:
URL: https://github.com/apache/systemds/pull/1672#discussion_r953935719
##########
src/main/java/org/apache/sysds/hops/DataOp.java:
##########
@@ -283,20 +308,28 @@ public Lop constructLops()
for (Entry<String, Integer> cur : _paramIndexMap.entrySet()) {
inputLops.put(cur.getKey(),
getInput().get(cur.getValue()).constructLops());
}
+ if(_ioGenRead)
+ inputLops.put("iogenformat",
_generateReaderOp.constructLops());
// Create the lop
switch(_op)
{
case TRANSIENTREAD:
- l = new Data(_op, null, inputLops, getName(),
null,
- getDataType(), getValueType(),
getFileFormat());
+ if(!_ioGenRead)
+ l = new Data(_op, null, inputLops,
getName(), null, getDataType(), getValueType(), getFileFormat());
+ else
+ l = new DataIOGen(_op, null, inputLops,
getName(), null, getDataType(), getValueType(), getIOGenFormat());
setOutputDimensions(l);
break;
case PERSISTENTREAD:
- l = new Data(_op, null, inputLops, getName(),
null,
- getDataType(), getValueType(),
getFileFormat());
-
l.getOutputParameters().setDimensions(getDim1(), getDim2(), _inBlocksize,
getNnz(), getUpdateType());
+ if(!_ioGenRead){
+ l = new Data(_op, null, inputLops,
getName(), null, getDataType(), getValueType(), getFileFormat());
+
l.getOutputParameters().setDimensions(getDim1(), getDim2(), _inBlocksize,
getNnz(), getUpdateType());
+ }
+ else
+ l = new DataIOGen(_op, null, inputLops,
getName(), null, getDataType(), getValueType(), getIOGenFormat());
Review Comment:
I think it would be good if you set the output parameters(dimensions) if
known. (like in the not IOGen above).
The previous (before your additions) is bad design setting the variables
after the call to the method, but it could be you do it inside your
constructor? if you do not i suggest to use the method on line 328.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
