DM-41711: Upgrade QuantumGraphExecutionReport to handle multiple overlapping graphs #426
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #426 +/- ##
==========================================
- Coverage 83.51% 83.22% -0.30%
==========================================
Files 97 99 +2
Lines 11427 11861 +434
Branches 2192 2280 +88
==========================================
+ Hits 9543 9871 +328
- Misses 1529 1603 +74
- Partials 355 387 +32 ☔ View full report in Codecov by Sentry. |
eigerx
force-pushed
the
tickets/DM-41711
branch
6 times, most recently
from
f6369f1
to
ff3811f
Compare
eigerx
force-pushed
the
tickets/DM-41711
branch
from
e2ce49c
to
3a95099
Compare
eigerx
force-pushed
the
tickets/DM-41711
branch
from
3a95099
to
101ccdb
Compare
eigerx
force-pushed
the
tickets/DM-41711
branch
4 times, most recently
from
e267079
to
7937a63
Compare
eigerx
force-pushed
the
tickets/DM-41711
branch
4 times, most recently
from
d099c8e
to
253edad
Compare
|
|
||
| from .graph import QuantumGraph | ||
|
|
||
| if TYPE_CHECKING: |
There was a problem hiding this comment.
Since only doing pass, does this if statement need to still be here?
| Parameters | ||
| ---------- | ||
| key : `QuantumKey` | ||
| The key used to refer to the node on the graph. |
There was a problem hiding this comment.
Return section on docstring? (Check every method) https://developer.lsst.io/python/numpydoc.html#naming-return-variables
| previous run associated with a quantum had the status FAILED, | ||
| and the status from the new graph reads SUCCESSFUL, we can | ||
| mark the overall quantum status as SUCCESSFUL and list the data_id | ||
| as RECOVERED. |
There was a problem hiding this comment.
Since this paragraph discusses the algorithm, I think it belongs in a note section.
See https://developer.lsst.io/python/numpydoc.html#extended-summary
| # if we also have logs, this is a success. | ||
| if log_dataset_run.produced: | ||
| quantum_run.status = QuantumRunStatus.SUCCESSFUL | ||
| # if we have metadata and no logs, this is a very rare |
There was a problem hiding this comment.
It may be just me, but I find it harder to read with the else comment before the else statement.
| # If the most recent graph's timestamp was earlier than any of the | ||
| # previous graphs, raise a RuntimeError. | ||
| if len(qgraphs) > 1: | ||
| for previous_graph in qgraphs[: count - 1]: |
There was a problem hiding this comment.
I don't understand this loop. If the qgraph list is in order, don't we just need to compare current qgraph with just previous qgraph (cause the previous qgraph was already checked with the other previous qgraphs and has to have the most recent time of the previous qgraphs)
| self.__add_new_graph(butler, qgraph) | ||
| output_runs.append(qgraph.metadata["output_run"]) | ||
| # If the user has not passed a `collections` variable | ||
| if not collections: |
There was a problem hiding this comment.
comment to explain why reversing runs?
…et- and quantum-run collecion pairs
Successful, no-work, and the weird ones we've already categorized are easy enough. But failed/blocked get tricky when you consider what else they might have been combined with.
… user to pass graphs in order
eigerx
force-pushed
the
tickets/DM-41711
branch
from
d0c7311
to
e5d3d75
Compare
Checklist
doc/changes