Repository navigation
Add enclose for the stages of nested loops - #248
Draft
SimonHeybrock wants to merge 1 commit into
Draft
SimonHeybrock wants to merge 1 commit into
SimonHeybrock wants to merge 1 commit into
Conversation
Starting point of the follow-up proposal for nested loops. This brings back enclose, its tests, the LoKI runs times banks validation, the nested-loops part of the user guide, and Stage.compute ignoring values for keys the stage does not use, as they were before they moved out of ADR 0003. The ADR and design document text about enclose comes back with them, to be moved into its own ADR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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
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.
Stacked on #245. Adds
enclose, which builds the stages of nested loops, such as detector banks within runs, from the inside out.Why
#245 handles nested loops with one stage per loop, the forwarded values derived from
Stage.frontierby hand (ADR 0003, "Nested loops"). That leaves a silent pitfall: a key that depends on the run but not on the bank, declared as an output of the bank stage, is pushed once per bank and counted several times without an error. It also takes a few lines of stage building per level, repeated by every nested driver (esssans runs times banks, Bifrost triplets times runs, theStreamProcessorcontext).enclose(pipeline, stages, inputs=...)puts stages inside a loop overinputs. It returns an outer stage that computes, once per iteration, the values the given stages held and that depend oninputs, followed by the given stages rebuilt to take those values as inputs. Nesting is the order of the calls. It rejects an output declared on a stage that does not vary in it.enclosewent through three designs (Aggregation,splitwith onePartper loop,enclose), which is why it was moved out of #245: the decision to replace map/reduce does not depend on it.Content
sciline.enclose, with tests intests/enclose_test.py.Stage.computeignores values for keys the stage does not use, so a driver passes each stage everything the stages of the enclosing loops returned. It still rejects a value for a key the stage holds or computes itself.loki_banks_validation.py: the esssans LoKI reduction over two runs times nine banks againstwith_banks(with_sample_runs(...)), with identical results.Status
Draft. The first commit restores the state before the split, so the
enclosetext is back in ADR 0003 and the design document. It moves to its own ADR 0004 once #245 has had a review round, since the nested-loops section of ADR 0003 decides what 0004 has to add.Test plan
tests/enclose_test.pyand the full suite pass, also with the dask environment.loki_banks_validation.pyrun locally against the reference: identical results.🤖 Generated with Claude Code