Conversation
Every generated POD after the first installs its predecessor in input slot 0, even when it imports no statements from that POD. Charge this slot in both partitioners, report total input-POD usage in diagnostics, and verify that materialization agrees with the solver.
Track external dependencies by builder input position instead of public-statement hash. Distinct input PODs can expose the same statement tree, so hash-based coalescing can undercount input slots and attach the wrong concrete POD during materialization.
External imports are already represented by assigned Open or synthetic nodes, which consume their statement-table slots. Only cross-POD chain imports create additional statements, so remove the duplicate external-import charge from the heuristic, MILP model, and diagnostics.
Add an external-opening-first topological ordering candidate so POD 0 can use its external input slots before later PODs reserve one for their predecessor. Mark frontend Open operations and synthetic republishes in the symbolic costs, retain the existing candidates, and cover a plan that drops from three PODs to two.
robknight
force-pushed
the
fix_too_many_input_pods
branch
from
August 27, 2026 14:16
77b1be6 to
25f086f
Compare
robknight
changed the title
ed255
approved these changes
Sep 7, 2026
ed255
left a comment
ed255
left a comment
Collaborator
There was a problem hiding this comment.
LGTM! I've left some minor comments / questions, please take a look before merging.
| pub struct OperationCost { | ||
| /// True for a statement copied directly from an external input POD. | ||
| #[serde(default)] | ||
| pub is_external_opening: bool, |
Collaborator
There was a problem hiding this comment.
Why is this boolean and the others are usize?
Since this is an operation, the usize fields will be 0 or 1, so they could be bool as well? But then I guess adding is easier if they are usize; yet this new field is bool.
| let n_ext_imports = self.external_imports.len() + self.scratch_new_ext_imports.len(); | ||
| let n_ext_pods = self.external_pods.len() + self.scratch_new_ext_pods.len(); | ||
| if !tree_imports_ok(n_producers, n_ext_imports, n_ext_pods, params) { | ||
| if !tree_imports_ok(self.a == 0, n_producers, n_ext_imports, n_ext_pods, params) { |
Collaborator
There was a problem hiding this comment.
could you rename this a field into something more meaningful?
| } | ||
| match input.dep_edges[statement].as_slice() { | ||
| [AbstractDep::External { pod, .. }] => Some(*pod), | ||
| _ => None, |
Collaborator
There was a problem hiding this comment.
is this case reachable? Because we just saw that is_external_opening = true. If it's unreachable I recommend using unreachable!() to be explicit about it.
Collaborator
There was a problem hiding this comment.
In fact, is input.costs[statement].is_external_opening fully coherent with input.dep_edges[statement].as_slice()? If so, wouldn't this be enough?
n external_opening_pod(input: &InputShape, statement: usize) -> Option<usize> {
match input.dep_edges[statement].as_slice() {
[AbstractDep::External { pod, .. }] => Some(*pod),
_ => None,
}
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Correct several related issues in how MultiPodBuilder accounts for and schedules external inputs.
Both the heuristic and MILP partitioners now use consistent input-POD and statement accounting. Diagnostics report total input-POD usage, and materialization asserts that its concrete input list agrees with the solver.