DM-55320: Add a feature to prune unanchored quanta in qgraph building#569
Conversation
There was a problem hiding this comment.
Looks good; only minor comments.
Can you confirm my understanding of how this relates to our earlier discussion on this?
-
When we talking about this at pair-programming, I was thinking that we would have to iterate between this and the existing skip/prune logic to ensure a self-consistent graph that satisfies all
adjustQuantumhooks. -
That's not actually needed because the logic here just removes all downstream quanta entirely, and hence remaining quanta's
adjustQuantuminputs would be unaffected. -
There is a hypothetical alternate version of this feature where you just prune the unanchored source quantum, and then let
adjustQuantumpruning act more selectively downstream. But what's here is actually a better fit for your use case. -
We would also need some kind of iteration if we accepted multiple (anchor, source) pairs, because those might interact. But you don't need that either.
Finally, one thing just to be aware of: like the existing pruning, this happens after the dimension records for the QG are fetched, and hence the QG will end up storing dimension records even for pruned quanta. This is usually a minor storage/memory inefficiency because the pruned fraction is small (and pruning the dimension records at the end is a denormalization problem that's usually a waste of time). But we've hit pathological QG builds in the past where the original graph is enormous and the pruned graph is small, and that could happen here too. But the best fix for this case it to constrain the original QG build better (this can really only happen with --dataset-query-constraint off and a weak --data-query).
|
I agree with all you said. My recollection from our pair-programming (though that was before I looked at Accepting multiple (anchor, source) pairs may still be a thing we want to do, but I feel reluctant to implement it before we have a concrete use case. Whether iterations are needed may depend on what kinds of task pairs we have in mind. |
9d0c75c to
8e6b8c5
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #569 +/- ##
==========================================
- Coverage 88.82% 88.79% -0.03%
==========================================
Files 160 161 +1
Lines 22357 22448 +91
Branches 2657 2673 +16
==========================================
+ Hits 19858 19933 +75
- Misses 1860 1869 +9
- Partials 639 646 +7 ☔ View full report in Codecov by Harness. |
7217076 to
0a8d050
Compare
This absorbs the functionality of ap_pipe's prune_orphan_preloads.py post-processing script into qgraph generation.
0a8d050 to
a2b1723
Compare
Checklist
package-docs builddoc/changes