Skip to content

perf: Use a single node to represent multiple shadow variables in graphs#1673

Merged
triceo merged 3 commits into
TimefoldAI:mainfrom
Christopher-Chianelli:perf/declarative-shadows-arb-graph
Jul 5, 2025
Merged

perf: Use a single node to represent multiple shadow variables in graphs#1673
triceo merged 3 commits into
TimefoldAI:mainfrom
Christopher-Chianelli:perf/declarative-shadows-arb-graph

Conversation

@Christopher-Chianelli

@Christopher-Chianelli Christopher-Chianelli commented Jul 3, 2025

Copy link
Copy Markdown
Contributor

When there is only a single declarative entity and all non-declarative parents go in the same direction (ex: previous), then some variables can share the same node in the graph. In particular, there is a static topological order when only the variables on a single entity is considered, and as long as the next declarative shadow variable does not have any new non-declarative parents (i.e. group), it can be grouped into the same node as the previous one. All nodes share the same node if no groups are used.

Also fixed a bug in the topological sort done to determine variable trigger order when groups are used (previously it added group variables, which caused it to create a cycle and make all topological orders valid).

When there is only a single declarative entity and all
non-declarative parents go in the same direction (ex: previous),
then some variables can share the same node in the graph. In
particular, there is a static topological order when only the
variables on a single entity is considered, and as long as
the next declarative shadow variable does not have any new
non-declarative parents (i.e. group), it can be grouped into
the same node as the previous one.

@triceo triceo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, small comments.

@triceo triceo added this to the v1.24.0 milestone Jul 4, 2025
@triceo

triceo commented Jul 4, 2025

Copy link
Copy Markdown
Collaborator

Oh and also, a green Sonar rating, please.

… used

- Also fixed a bug in the topological sort done to determine variable
  trigger order when groups are used (previously it added group variables,
  which caused it to create a cycle and make all topological orders
  valid).
@triceo triceo merged commit 6c3c668 into TimefoldAI:main Jul 5, 2025
42 of 43 checks passed
@sonarqubecloud

sonarqubecloud Bot commented Jul 5, 2025

Copy link
Copy Markdown

@Christopher-Chianelli Christopher-Chianelli deleted the perf/declarative-shadows-arb-graph branch May 14, 2026 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Investigate and improve runtime performance of Declarative Shadow Variables

2 participants