splitting_tools.subset_merger() merges through _try_to_merge_subsets(), which requires the two subsets to be identical in all but one dimension and, in that dimension, exactly adjacent (end1 + 1 == start2, ends inclusive). Overlapping subsets are not merged.
subset_merger(['0:5', '5:15']) -> ['0:15'] # adjacent
subset_merger(['0:10', '5:15']) -> ['0:10', '5:15'] # overlapping, not merged
That is correct for its stated job — describing how to split a node, where overlapping pieces would be ambiguous. It is wrong as a coverage test. Asking "do these consumers together read all of 0:15?" via
any(m.covers(subset) for m in subset_merger(consumer_subsets))
answers False for reads of 0:10 and 5:15, which plainly do cover it.
Why it matters
Consumers reading overlapping regions is the normal case for a stencil reading a halo, so a coverage test built on subset_merger rejects the common case. It fails in the safe direction — a transformation declines rather than miscompiling — which is exactly why it is easy to miss: nothing breaks, the optimization silently does not fire.
This bit the multi-producer fragment check in #2764, where a fully read fragment was rejected.
Suggestion
subset_merger should probably keep its current meaning. What is missing is a sibling that also joins overlapping and contained subsets, for callers asking about coverage rather than about a split. #2764 carries a local implementation (_merge_overlapping_subsets / _try_to_merge_overlapping_subsets) that was deliberately kept private to avoid changing subset_merger's semantics mid review — happy to move it into splitting_tools.py if that is where you would want it.
splitting_tools.subset_merger()merges through_try_to_merge_subsets(), which requires the two subsets to be identical in all but one dimension and, in that dimension, exactly adjacent (end1 + 1 == start2, ends inclusive). Overlapping subsets are not merged.That is correct for its stated job — describing how to split a node, where overlapping pieces would be ambiguous. It is wrong as a coverage test. Asking "do these consumers together read all of
0:15?" viaanswers False for reads of
0:10and5:15, which plainly do cover it.Why it matters
Consumers reading overlapping regions is the normal case for a stencil reading a halo, so a coverage test built on
subset_mergerrejects the common case. It fails in the safe direction — a transformation declines rather than miscompiling — which is exactly why it is easy to miss: nothing breaks, the optimization silently does not fire.This bit the multi-producer fragment check in #2764, where a fully read fragment was rejected.
Suggestion
subset_mergershould probably keep its current meaning. What is missing is a sibling that also joins overlapping and contained subsets, for callers asking about coverage rather than about a split. #2764 carries a local implementation (_merge_overlapping_subsets/_try_to_merge_overlapping_subsets) that was deliberately kept private to avoid changingsubset_merger's semantics mid review — happy to move it intosplitting_tools.pyif that is where you would want it.