Skip to content

Prototype: demand-driven generics without map-time forward instantiation #238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
SimonHeybrock wants to merge 4 commits into from
Closed

Prototype: demand-driven generics without map-time forward instantiation #238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8bc7a91
Drop map-time forward instantiation via deferred mapped labeling
SimonHeybrock Aug 21, 2026
4a1afac
Record Q1 spike results in design document
SimonHeybrock Aug 21, 2026
4cbf9fd
Delete forward chaining; adapt to strict node-name uniqueness
SimonHeybrock Aug 21, 2026
e6084bb
Apply automatic formatting
pre-commit-ci-lite[bot] Aug 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 57 additions & 3 deletions docs/developer/architecture-and-design/demand-driven-generics.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,49 @@ Coupling with Q3: without forward chaining, the unseeded forward pass behind
`output_keys()` / no-arg `visualize()` loses its engine, which strengthens the
case for rule-graph-first inspection.

**Spike results (2026-08-21;
[scipp/cyclebane#32](https://github.com/scipp/cyclebane/pull/32) and sciline
branch `235-deferred-mapped-labels`): the proposal works.** Cyclebane keeps
plain node names and derives all mapped labeling inside `to_networkx()`; all
143 cyclebane tests pass with identical compiled output, and all 245 sciline
tests passed *without modification* against it. Sciline then dropped the
`map()`-time forward hook and the whole seed-restriction machinery — forward
chaining now exists only behind `output_keys()`, pending Q3. Resolution of the
open points:

- *Shadowing idioms are the irreducible core of the old labeling.*
`reduce(name=C, index='x')` and the documented
`pipeline[C] = pipeline[C].map(...).reduce(func=merge)` pattern give the
reduce result the same public name as the mapped node it reduces; the old
global relabeling made that work implicitly (plain `C` vs `MappedNode(C)`).
With plain names this is a genuine collision. A first iteration kept the
dual identity at exactly these seams via scoped `MappedNode` aliases;
**decision (SH, 2026-08-21): forbid the shadowing idioms instead** — node
names are strictly unique, `reduce` and branch assignment raise clear errors
on collisions, and callers use distinct names (e.g. a `MaskPerFile`-style
name for the mapped node, distinct from the merged result). The
subgraph-reuse requirement that motivated the graft idiom should be
addressed explicitly, e.g. via a future `Graph.rename`. This removes the
alias mechanism, the `named_indices` accessor and the multiple-candidates
disambiguation in `get_mapped_node_names` (`index_names=` is now pure
validation), and turns the pre-existing silent self-loop bug into an error.
- *`reduce(key=None)`*: the sink is resolved among nodes present in the graph;
reducing a sink that would come from a rule requires an explicit `key`,
which sciline treats as a demand (backward instantiation). Two tests were
updated to pass an explicit key.
- *Mapped roots* are treated as satisfied keys; backward instantiation never
provides them.
- *Index-order compatibility* held: the derivation reproduces the per-node
index tuple order of incremental relabeling (later maps first, intra-map
order preserved, groupby extras appended), pinned by the existing tests.
- *Bug found on cyclebane `main`*: `graph1['c'] = graph2['d']` merges the
branch's own `'c'` with the destination during sink-renaming, creating a
self-loop (reproducible on `main`, invisible because nothing topologically
sorts the stored graph). Fixed by the strict collision error above.
Relatedly, `__setitem__` now permits grafting a mapped branch at a *new*
node name (previously impossible); the mapped/unmapped consistency check
only applies when replacing an existing branch.

### Q2: Coherence instead of last-wins among rules — decided, implemented

The prototype initially resolved multiple matching rules by "latest registered
Expand Down Expand Up @@ -298,8 +341,15 @@ This is likely the easiest of the three (rendering code plus changed
defaults). The one genuinely breaking decision is `output_keys()`: does it
return concrete keys (forward-expanded, status quo), patterns for rules plus
concrete sinks (breaking for `compute(pl.output_keys())`), or does the rule
view get a separate accessor while `output_keys()` keeps its meaning? Open;
affects `visualize()`'s no-argument default via `tp=self.output_keys()`.
view get a separate accessor while `output_keys()` keeps its meaning?

**Prototype decision (SH, 2026-08-21): take the choice that simplifies most.**
Forward chaining (`_instantiate_forward`, `forward_bindings`) is deleted; the
engine is backward-only. `output_keys()` returns concrete graph sinks plus the
return-type *patterns* of rules not consumed by other rules (consumption
approximated by pattern-origin comparison). No-argument `visualize()` shows
the concrete part of the graph, since patterns cannot be demanded. Rule-graph
*rendering* remains future work.

## Decision log

Expand All @@ -310,7 +360,11 @@ affects `visualize()`'s no-argument default via `tp=self.output_keys()`.
| 2026-08-21 | Specialized-provider-shadows-generic accepted as a semantic change; generic-replaces-specialized not worth preserving. | #237, this doc |
| 2026-08-21 | Q2 decided: three-tier coherence (replace equal patterns, most-specific wins, incomparable overlap errors) instead of last-wins. Implemented. | #237 |
| open | Single mechanism (#237) vs. coexistence (#236) vs. separate class. | discussion |
| open | Q1 (forward chaining vs. deferred mapped-labeling in cyclebane), Q3 (rule-graph inspection). | this doc |
| 2026-08-21 | Q1 spike validates deferred mapped-labeling: both suites green, sciline `map()` hook and seeding removed. | scipp/cyclebane#32, branch `235-deferred-mapped-labels` |
| 2026-08-21 | Shadowing idioms (`reduce(name=<reduced node>)`, `pipeline[C] = pipeline[C].map(...).reduce(...)`) are forbidden instead of alias-supported; node names are strictly unique. Subgraph-reuse to be addressed explicitly (e.g. future `Graph.rename`). | scipp/cyclebane#32 |
| 2026-08-21 | Explicit `key` required in `reduce` when the reduced sink would come from a rule; concrete pipelines are unaffected. | #238 |
| 2026-08-21 | Q3 resolved for the prototype via the simplest path: forward chaining deleted, `output_keys()` lists unconsumed rule patterns, no-arg `visualize()` shows the concrete part. Rule-graph rendering is future work. | #238 |
| open | Adopt the Q1/Q3 end state into #237 and land scipp/cyclebane#32. | discussion |

## References

Expand Down
42 changes: 0 additions & 42 deletions src/sciline/_unification.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,45 +145,3 @@ def match_return(template: Provider, key: Key) -> Provider | None:
if unify(template.deduce_key(), key, bound):
return template.bind_type_vars(bound)
return None


def forward_bindings(
template: Provider, known_keys: Iterable[Key]
) -> Generator[tuple[dict[TypeVar, Key], frozenset[Key]], None, None]:
"""Enumerate complete bindings of a template's TypeVars from known keys.

Each generic argument of the template is unified with each known key;
consistent combinations of the resulting bindings that bind all type
variables of the template are yielded, together with the set of known keys
that produced them. Arguments that match no known key are left unmatched,
i.e., a binding is complete as long as the *other* arguments determine all
type variables.
"""
typevars = find_all_typevars(template.deduce_key())
patterns = [p for p in template.arg_spec.keys() if find_all_typevars(p)]
options = []
for pattern in patterns:
matches: list[tuple[dict[TypeVar, Key], Key | None]] = [({}, None)]
for key in known_keys:
bound: dict[TypeVar, Key] = {}
if unify(pattern, key, bound):
matches.append((bound, key))
options.append(matches)
seen = set()
for combo in itertools.product(*options):
merged: dict[TypeVar, Key] = {}
if not all(_merge(merged, bound) for bound, _ in combo):
continue
if set(merged) != typevars:
continue
used = frozenset(key for _, key in combo if key is not None)
if (fingerprint := (frozenset(merged.items()), used)) not in seen:
seen.add(fingerprint)
yield merged, used


def _merge(target: dict[TypeVar, Key], bound: dict[TypeVar, Key]) -> bool:
for tv, key in bound.items():
if target.setdefault(tv, key) != key:
return False
return True
98 changes: 38 additions & 60 deletions src/sciline/data_graph.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
_bind_free_typevars,
)
from ._unification import (
_pattern_origin_and_args,
find_all_typevars,
forward_bindings,
match_return,
parameterize,
subsumes,
Expand Down Expand Up @@ -149,7 +149,7 @@ def _register_template(self, provider: Provider) -> None:

Generic providers are instantiated by unifying their type patterns
with the concrete keys demanded from the graph, see
:py:meth:`_instantiate_backward` and :py:meth:`_instantiate_forward`.
:py:meth:`_instantiate_backward`.
"""
spec = provider.arg_spec.map_keys(parameterize)
provider = Provider(func=provider.func, arg_spec=spec, kind=provider.kind)
Expand Down Expand Up @@ -230,7 +230,10 @@ def is_dominated(pattern: Key) -> bool:

def _satisfied(self, key: Key) -> bool:
graph = self.underlying_graph
return key in graph and bool(graph.nodes[key].keys() & _providing_attrs)
if key in graph and bool(graph.nodes[key].keys() & _providing_attrs):
return True
# Mapped roots receive their values from the mapping.
return key in self._cbgraph.value_keys

def _instantiate_backward(self, keys: Iterable[Key]) -> None:
"""Instantiate templates for demanded keys and their dependencies."""
Expand All @@ -250,56 +253,27 @@ def _instantiate_backward(self, keys: Iterable[Key]) -> None:
if key in self.underlying_graph:
stack.extend(self.underlying_graph.predecessors(key))

def _instantiate_forward(self, seeds: Iterable[Key] | None = None) -> None:
"""Instantiate templates whose arguments unify with concrete keys.
def _template_output_patterns(self) -> list[Key]:
"""Return-type patterns of generic providers not consumed by templates.

If ``seeds`` is given, only instantiations consuming a seed key or a
key derived from one are created; other keys may still contribute to
bindings. Otherwise all complete bindings from the present concrete
keys are instantiated. Runs to a fixed point since instantiated
providers introduce new keys.
Whether one generic provider consumes another's output cannot be
decided exactly without instantiation; consumption is approximated by
comparing pattern origins.
"""
derived = None if seeds is None else set(seeds)
done: set[Key] = set()
while True:
known = set(self.underlying_graph.nodes)
if derived is not None:
known |= derived
candidates: list[Key] = []
for template in self._templates:
if not isinstance(template, Provider):
continue
for bound, used in forward_bindings(template, known):
if derived is not None and not (used & derived):
continue
candidates.append(_bind_free_typevars(template.deduce_key(), bound))
progressed = False
for key in candidates:
if key in done or self._satisfied(key):
continue
done.add(key)
# Resolve via _matching_template so that the latest-registered
# template wins, which may differ from the candidate's origin.
resolved = self._matching_template(key)
if isinstance(resolved, _TemplateValue):
self[key] = resolved.value
elif resolved is not None:
self.insert(resolved)
else:
continue
progressed = True
if derived is not None:
derived.add(key)
if not progressed:
# Values for dangling inputs of instantiated providers. Only
# applied where the latest matching template is a value;
# provider matches are left for backward instantiation.
for key in list(self.underlying_graph.nodes):
if not self._satisfied(key) and isinstance(
resolved := self._matching_template(key), _TemplateValue
):
self[key] = resolved.value
return
providers = [t for t in self._templates if isinstance(t, Provider)]
consumed = {
origin
for template in providers
for arg in template.arg_spec.keys()
if find_all_typevars(arg)
and (origin := _pattern_origin_and_args(arg)[0]) is not None
}
return [
pattern
for template in providers
if _pattern_origin_and_args(pattern := template.deduce_key())[0]
not in consumed
]

def __setitem__(self, key: Key, value: DataGraph | Any) -> None:
"""
Expand Down Expand Up @@ -349,13 +323,10 @@ def map(self: T, node_values: dict[Key, Any]) -> T:
:
A new graph with mapped nodes.
"""
graph = self
if self._has_templates:
# Mapping duplicates dependents of the mapped nodes, so generic
# providers must be instantiated first.
graph = self.copy()
graph._instantiate_forward(node_values.keys())
return graph._from_cyclebane(graph._cbgraph.map(node_values))
# Note that dependents of the mapped nodes need not exist yet: which
# nodes carry which indices is derived at task-graph build time, so
# providers instantiated on demand after mapping are handled correctly.
return self._from_cyclebane(self._cbgraph.map(node_values))

def reduce(self: T, *, func: Callable[..., Any], **kwargs: Any) -> T:
"""Reduce the outputs of a mapped graph into a single value and provider.
Expand All @@ -377,8 +348,15 @@ def reduce(self: T, *, func: Callable[..., Any], **kwargs: Any) -> T:
# Note that the type hints of `func` are not checked here. As we are explicit
# about the modification, this is in line with __setitem__ which does not
# perform such checks and allows for using generic reduction functions.
return self._from_cyclebane(
self._cbgraph.reduce(attrs={'reduce': func}, **kwargs)
graph = self
if (key := kwargs.get('key')) is not None and self._has_templates:
# The reduced key is a demand; instantiate providers for it. Without
# an explicit key, only nodes present in the graph are considered
# when determining the sink to reduce.
graph = self.copy()
graph._instantiate_backward((key,))
return graph._from_cyclebane(
graph._cbgraph.reduce(attrs={'reduce': func}, **kwargs)
)

def to_networkx(self) -> nx.DiGraph:
Expand Down
52 changes: 20 additions & 32 deletions src/sciline/pipeline.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from networkx.algorithms.simple_paths import all_simple_paths

from ._provider import Provider, ToProvider
from ._unification import find_all_typevars
from ._utils import key_name
from .data_graph import DataGraph, to_task_graph
from .display import pipeline_html_repr
Expand Down Expand Up @@ -215,7 +216,9 @@ def visualize(
Keyword arguments passed to :py:class:`graphviz.Digraph`.
"""
if tp is None:
tp = self.output_keys()
# Generic return-type patterns cannot be demanded, only their
# instantiations can; visualize the concrete part of the graph.
tp = tuple(key for key in self.output_keys() if not find_all_typevars(key))
return self.get(tp, handler=HandleAsComputeTimeException()).visualize(
compact=compact,
mode=mode,
Expand Down Expand Up @@ -342,16 +345,15 @@ def _repr_html_(self) -> str:
return pipeline_html_repr(nodes)

def output_keys(self) -> tuple[Key, ...]:
"""Returns the keys that are not inputs to any other providers."""
graph = self
if self._has_templates:
# Instantiate generic providers derivable from the present concrete
# keys so that their outputs are included.
graph = self.copy()
graph._instantiate_forward()
"""Returns the keys that are not inputs to any other providers.

For generic providers the (uninstantiated) return-type pattern is
included, unless another generic provider consumes it.
"""
sink_nodes = [
node for node, degree in graph.underlying_graph.out_degree if degree == 0
node for node, degree in self.underlying_graph.out_degree if degree == 0
]
sink_nodes += self._template_output_patterns()
return tuple(sorted(sink_nodes, key=key_name))


Expand Down Expand Up @@ -390,35 +392,23 @@ def get_mapped_node_names(
base_name:
The base name of the mapped node to get the names for.
index_names:
Specifies the names of the indices of the mapped node. If not given this is
inferred from the graph, but the argument may be required to disambiguate
multiple mapped nodes with the same name.
If given, must match the index names of the mapped node.

Returns
-------
:
The series of node names corresponding to the mapped node.
"""
pd = _import_pandas("sciline.get_mapped_node_names")
from cyclebane.graph import IndexValues, MappedNode, NodeName

candidates = [
node
for node in graph.underlying_graph.nodes
if isinstance(node, MappedNode) and node.name == base_name
]
if len(candidates) == 0:
from cyclebane.graph import IndexValues, NodeName

node_indices = graph._cbgraph.node_indices(base_name)
if not node_indices or (
index_names is not None and set(index_names) != set(node_indices)
):
raise ValueError(f"'{base_name}' is not a mapped node.")
if index_names is not None:
candidates = [
node for node in candidates if set(node.indices) == set(index_names)
]
if len(candidates) > 1:
raise ValueError(
f"Multiple mapped nodes with name '{base_name}' found: {candidates}"
)

index_names = tuple(reversed(candidates[0].indices))
index_names = tuple(reversed(node_indices))
indices = {name: idx for name, idx in graph.indices.items() if name in index_names}

index = pd.MultiIndex.from_product(indices.values(), names=index_names)
Expand Down Expand Up @@ -454,9 +444,7 @@ def compute_mapped(
base_name:
The base name of the mapped node to get the names for.
index_names:
Specifies the names of the indices of the mapped node. If not given this is
inferred from the graph, but the argument may be required to disambiguate
multiple mapped nodes with the same name.
If given, must match the index names of the mapped node.

Returns
-------
Expand Down
Loading
Loading