feat: support sourcing declarative shadow variables from planning list variable elements#2506
Open
fodzal wants to merge 3 commits into
Open
feat: support sourcing declarative shadow variables from planning list variable elements#2506fodzal wants to merge 3 commits into
fodzal wants to merge 3 commits into
Conversation
…t variable elements An entity declaring a @PlanningListVariable can now use a @Shadowsources path such as "values[].endTime" to declare that one of its declarative shadow variables depends on a declarative shadow variable of the list's elements, e.g. a vehicle-level end time aggregated from its visits. Previously, this dependency was inexpressible: "values[].endTime" failed fast (accesses a collection via a variable) and "lastValue.endTime" failed fast as well (declarative accessed from declarative). The closest available spelling, a bare list variable source such as @Shadowsources("values"), was accepted and documented but silently broken: it was computed once when the variable reference graph was built and never retriggered during solving. - RootVariableSource accepts a [] segment on a genuine list variable declared on the root entity, producing the new ParentVariableType.LIST_ELEMENT. The path must end on a declarative shadow variable of the element (optionally reached via a fact). - The initial fan-in edges (element.shadow -> owner.shadow) are created from the list's contents when the graph is built. - List variable change events are now forwarded to the declarative shadow variable session, which incrementally adds/removes the fan-in edges for the changed range and always marks the owner variable changed, so the aggregate also reacts to reorders and removals. - GraphStructure classifies LIST_ELEMENT sources as arbitrary, since the graph's edges change with the list's contents. - Bare list variable sources, e.g. @Shadowsources("values"), now fail fast with a message pointing to the new element syntax, and the mention of @PlanningListVariable as a valid source was removed from the @ShadowVariable javadoc, as they gave stale values that even FULL_ASSERT could not detect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ange The element's declarative shadow variable depends on a genuine basic variable and the element has no ordering shadow variables, proving the aggregate is retriggered through the element edge alone.
fodzal
marked this pull request as ready for review
July 15, 2026 15:51
fodzal
requested review from
Christopher-Chianelli,
TomCools and
triceo
as code owners
July 15, 2026 15:51
Christopher-Chianelli
left a comment
Contributor
There was a problem hiding this comment.
I will do my own testing of this to verify correctness. Some comments.
- Remove requiresListVariableChangeEvents(); list variable events are forwarded unconditionally, like basic variable events. - Rename ListElementEdgeMaintainer to ListElementSourceLocator, and the related field and method accordingly. - Rename the elements parameter to elementList. - Remove comments that stated the obvious. - Rewrite the incremental tests with MoveTester. - Use forEachIncludingUnassigned/forEach in the test constraint provider. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
Thanks @Christopher-Chianelli, I have done the required changes. I have tested that on a simplified model (VRP with sequential vehicles), it works well. |
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.
What
Follow-up to #2469. An entity declaring a
@PlanningListVariablecan now use a@ShadowSourcespath such as"visits[].endTime"to depend on a declarative shadowvariable of the list variable's elements:
This is the capability discussed in the #2469 review: accessing the declarative shadow
variables of entities inside the
@PlanningListVariable, with the edges of the graphcreated and updated as the list variable changes. The fail-fast added by #2469 now points
users to this syntax.
How
RootVariableSourceaccepts a[]segment on a genuine list variable declared on theroot entity, producing the new
ParentVariableType.LIST_ELEMENT. The path must end on adeclarative shadow variable of the element (optionally reached via a fact).
element.shadowVariable -> entity.shadowVariable) are created from thelist variable's contents when the graph is built. They are registered as initial dynamic
edges, not fixed edges, so a dependency loop through them is treated as breakable by the
solver instead of failing fast.
before/afterListVariableChangedevents are forwarded to the declarative shadowvariable session, which incrementally removes/adds the fan-in edges for the changed
range and always marks the target variable as changed (covering removals and
reorderings). Models without list element sources skip this entirely.
once per move, after all its elements' shadow variables have settled.
GraphStructureclassifies models with list element sources as arbitrary, since thegraph's edges change during solving; the grouped-updater decision is static for the same
reason.
Testing
values[].shadowVarforms and the new fail-fasts(non-declarative element variable, list variable reached via another member) in
RootVariableSourceTest.ListElementShadowVariableTest:SolutionManager.updateShadowVariables(includingempty lists and mutations between calls) and two
FULL_ASSERTsolver runs, one from anuninitialized solution and one from an initialized solution (exercising the initial
edge seeding).
testdomain/shadow/list_elementdomain is used instead of extendingsimple_list, because adding a list element source there would reclassify it away fromSINGLE_DIRECTIONAL_PARENTand lose that structure's coverage.coretest suite passes.