-
Notifications
You must be signed in to change notification settings - Fork 3
Feature/Include const_eval layer when flattening #82
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.
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
Changes from 4 commits
20d195f
60c609d
2f4c373
47a145a
bcd0353
320fce4
0681463
fba6df4
d373059
6bce35b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -52,6 +52,7 @@ import {getLayoutGraph} from './graph_layout'; | |||||||||
| import {updateProcessingProgress} from './utils'; | ||||||||||
|
|
||||||||||
| const CONST_VALUE_REGEX = /dense<([^>]*)>/; | ||||||||||
| const CONST_EVAL_LAYER_NAME = 'const_eval'; | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * A class that processes a given `Graph` into a `ModelGraph`. | ||||||||||
|
|
@@ -99,6 +100,10 @@ export class GraphProcessor { | |||||||||
| ProcessingLabel.PROCESSING_LAYER_NAMESPACES, | ||||||||||
| ); | ||||||||||
|
|
||||||||||
| if (this.flattenLayers) { | ||||||||||
| this.addSeparateConstEvalLayer(modelGraph); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| this.generateLayoutGraphConnections(modelGraph); | ||||||||||
| updateProcessingProgress( | ||||||||||
| this.paneId, | ||||||||||
|
|
@@ -353,63 +358,7 @@ export class GraphProcessor { | |||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Find group nodes that only have one single op node as its child. For | ||||||||||
| // these nodes, remove the group node and move the child op node up a level | ||||||||||
| // from its namespace. | ||||||||||
| // | ||||||||||
| // Repeatedly do this until no such nodes are found. | ||||||||||
| if (!this.keepLayersWithASingleChild) { | ||||||||||
| while (true) { | ||||||||||
| let numNodeProcessed = 0; | ||||||||||
| for (const node of modelGraph.nodes) { | ||||||||||
| if (!isGroupNode(node)) { | ||||||||||
| continue; | ||||||||||
| } | ||||||||||
| if (node.nsChildrenIds != null && node.nsChildrenIds.length === 1) { | ||||||||||
| const opNode = modelGraph.nodesById[node.nsChildrenIds[0]]; | ||||||||||
| if (isOpNode(opNode)) { | ||||||||||
| numNodeProcessed++; | ||||||||||
| // Delete group node. | ||||||||||
| const index = modelGraph.nodes.indexOf(node); | ||||||||||
| if (index >= 0) { | ||||||||||
| modelGraph.nodes.splice(index, 1); | ||||||||||
| } | ||||||||||
| delete modelGraph.nodesById[node.id]; | ||||||||||
|
|
||||||||||
| // Move op node up one level in namespace. | ||||||||||
| const ns = opNode.namespace; | ||||||||||
| const parts = this.getNonEmptyNamespaceComponents(ns); | ||||||||||
| parts.pop(); | ||||||||||
| opNode.namespace = parts.join('/'); | ||||||||||
| opNode.savedNamespace = opNode.namespace; | ||||||||||
| opNode.level = parts.length; | ||||||||||
| opNode.nsParentId = node.nsParentId; | ||||||||||
|
|
||||||||||
| // Update root node if necessary. | ||||||||||
| const indexInRootNodes = modelGraph.rootNodes.indexOf(node); | ||||||||||
| if (indexInRootNodes >= 0) { | ||||||||||
| modelGraph.rootNodes.splice(indexInRootNodes, 1); | ||||||||||
| modelGraph.rootNodes.push(opNode); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Remove this node from its NS parent node's nsChildrenIds, and add | ||||||||||
| // the op node to it. | ||||||||||
| if (node.nsParentId) { | ||||||||||
| const nsParent = modelGraph.nodesById[ | ||||||||||
| node.nsParentId | ||||||||||
| ] as GroupNode; | ||||||||||
| const index = nsParent.nsChildrenIds!.indexOf(node.id); | ||||||||||
| nsParent.nsChildrenIds!.splice(index, 1); | ||||||||||
| nsParent.nsChildrenIds!.push(opNode.id); | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| if (numNodeProcessed === 0) { | ||||||||||
| break; | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| this.removeSingleChildGroupNodes(modelGraph); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
|
|
@@ -522,6 +471,195 @@ export class GraphProcessor { | |||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Finds all nodes with "const_eval" in their namespace and reorganizes them | ||||||||||
| * under a separate "const_eval" root group, preserving their hierarchy. | ||||||||||
| */ | ||||||||||
| addSeparateConstEvalLayer(modelGraph: ModelGraph) { | ||||||||||
| const constEvalNodes: OpNode[] = []; | ||||||||||
|
|
||||||||||
| // Get all nodes that have "const_eval" in their namespace | ||||||||||
|
dcblundell marked this conversation as resolved.
Outdated
|
||||||||||
| for (const node of modelGraph.nodes) { | ||||||||||
| if (isOpNode(node) && node.fullNamespace?.includes(CONST_EVAL_LAYER_NAME)) { | ||||||||||
|
dcblundell marked this conversation as resolved.
Outdated
|
||||||||||
| constEvalNodes.push(node); | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if (constEvalNodes.length === 0) { | ||||||||||
| return; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Create the const_eval root group node | ||||||||||
| const constEvalGroupId = `${CONST_EVAL_LAYER_NAME}___group___`; | ||||||||||
| const constEvalGroupNode: GroupNode = { | ||||||||||
| nodeType: NodeType.GROUP_NODE, | ||||||||||
| id: constEvalGroupId, | ||||||||||
| namespace: '', | ||||||||||
| label: CONST_EVAL_LAYER_NAME, | ||||||||||
| level: 0, | ||||||||||
| expanded: false, | ||||||||||
| nsChildrenIds: [], | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| modelGraph.nodes.push(constEvalGroupNode); | ||||||||||
| modelGraph.nodesById[constEvalGroupId] = constEvalGroupNode; | ||||||||||
|
|
||||||||||
| const seenNamespaces = new Set<string>(); | ||||||||||
|
|
||||||||||
| // Remove nodes from old parents and update their namespaces | ||||||||||
| for (const node of constEvalNodes) { | ||||||||||
| this.removeFromParentOrRoot(modelGraph, node); | ||||||||||
|
|
||||||||||
| // Restore full namespace hierarchy | ||||||||||
| node.namespace = node.fullNamespace || node.namespace; | ||||||||||
|
||||||||||
| node.namespace = node.fullNamespace || node.namespace; | |
| node.namespace = node.fullNamespace || node.namespace; | |
| node.savedNamespace = node.namespace; |
Copilot
AI
Feb 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a node has namespace exactly equal to "const_eval", then getAncestorNamespaces will return ["const_eval"], and the loop at lines 501-503 will call createGroupNodeForNamespace with namespace "const_eval". This will try to create a group node with ID "const_eval___group___", which is the same ID as the root const_eval group node created at line 474. This causes a duplicate node in modelGraph.nodes and overwrites the original node in modelGraph.nodesById.
To fix this, add "const_eval" to seenNamespaces before the loop:
seenNamespaces.add(CONST_EVAL_LAYER_NAME);
after line 488.
Copilot
AI
Feb 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a filter to handle any potential undefined nodes. While createGroupNodeForNamespace should create all nodes in seenNamespaces, adding a safety check would make the code more robust:
.filter(node => node != null) after the map operation.
| ...Array.from(seenNamespaces).map(ns => modelGraph.nodesById[this.getGroupNodeIdFromNamespace(ns)]), | |
| ...Array.from(seenNamespaces) | |
| .map(ns => modelGraph.nodesById[this.getGroupNodeIdFromNamespace(ns)]) | |
| .filter(node => node != null), |
Uh oh!
There was an error while loading. Please reload this page.