DM-21390: Add a show option to examine a qgraph #27
Conversation
| def _showWorkflow(self, graph, args): | ||
| """Print quanta information and dependency to stdout | ||
|
|
||
| The input and pridicted output URIs based on the Butler repo are printed. |
| node = QuantumGraphTaskNodes(taskNodes.taskDef, [quantum], | ||
| quantum.initInputs, quantum.outputs) | ||
| qgraph = QuantumGraph() | ||
| qgraph.append(node) |
There was a problem hiding this comment.
Combine two lines into a shorter qgraph = QuantumGraph([node])?
Maybe we should also move this operation that splits one QGraph into multiple single-quantum QGraphs into the Graph class itself, if that is a useful operation by itself.
There was a problem hiding this comment.
I'm not sure how useful it is beyond small test workflows, but moving it to the QuantumGraph class makes sense, and indeed seems safer with potential implementation changes of the class itself.
I created lsst/pipe_base#103 for it.
| hash2ParentTask = {} | ||
| for taskNodes in graph: | ||
| for quantum in taskNodes.quanta: | ||
| iq += 1 |
There was a problem hiding this comment.
I'd probably make iteration in one loop:
for iq, (taskDef, quantum) in enumerate(graph.quanta()):
There is a chance that implementation of the QuantumGraph will change (making it a subclass of a list was probably wrong decision), quanta() method should be safer w.r.t. potential changes.
There was a problem hiding this comment.
quanta() is a useful method. Thanks for bringing it up.
| iq += 1 | ||
| shortname = taskNodes.taskDef.taskName.split('.')[-1] | ||
| print("Quantum {}: {}".format(iq, shortname)) | ||
| if args.save_qgraph: |
There was a problem hiding this comment.
Hmm, reusing the same --save-qgraph can have a surprising effect, that will also save full graph and can inadvertently override existing data if you are not careful. It may be better to introduce new special option like --save-single-quanta which takes the file name pattern (e.g. --save-single-quanta=single-quantum-{}.pickle). I'd probably made this new option independent of --show=workflow or explicitly mention that it only works with --show=workflow if you prefer current way.
There was a problem hiding this comment.
Good idea. Done.
Related to this, about the QuantumGraph.quanta() method, for one same given QuantumGraph, I understand quanta are returned in unspecified order (as said in its doc), but is it okay to assume that order is the same in multiple invocations? (Looks fine in the current list implementation, but maybe not guaranteed in the future?)
| hash2ParentTask[dhash] = iq | ||
|
|
||
| iq = 0 | ||
| uses = [] |
There was a problem hiding this comment.
I'd probably use set instead of list here, searching in a set should be faster.
hsinfang
force-pushed
the
tickets/DM-21390
branch
2 times, most recently
from
b9b6743
to
bb70868
Compare
hsinfang
force-pushed
the
tickets/DM-21390
branch
from
bb70868
to
90e8d55
Compare
No description provided.