Refactors and fixes #131
Refactors and fixes #131
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughRefactors sign-propagation/type-inference to an abstract API; improves integer immediate handling; migrates iNoNorm from Generic to Snitch (parser, layer, checker, template, bindings, platform); renames many template refs from Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as TypeInferenceCaller
participant SignProp as SignPropTypeChecker (abstract)
participant Impl as ConcreteChecker
Caller->>SignProp: typeInferOutput(inputs, opRep)
activate SignProp
Note right of SignProp #E8F6FF: Delegates to abstract helpers
SignProp->>SignProp: _inferNumLevels(inputs, opRep)
SignProp->>SignProp: _inferSignedness(inputs, opRep)
deactivate SignProp
alt Snitch iNoNorm
Impl->>Impl: _inferNumLevels -> computed levels
Impl->>Impl: _inferSignedness -> computed signedness
else other implementations
Impl->>Impl: other logic
end
sequenceDiagram
participant Parser as DerivedParser
participant Base as Base.parseNode
participant Validator as GeometryValidator
Parser->>Base: super().parseNode(node)
Base-->>Parser: bool valid
alt not valid
Parser-->>Parser: return False
else valid
Parser->>Validator: validate pads/kernel/strides lengths
Validator-->>Parser: pass/fail
alt pass
Parser-->>Parser: populate operatorRepresentation / return True
else fail
Parser-->>Parser: return False
end
end
sequenceDiagram
participant Template as GEMM Template
participant Ref as ReferenceDecl
participant Kernel as GEMM Kernel
Template->>Ref: declare ref_${nodeName}_${A/B/C/data_out}
alt batched inputs
Template->>Template: conditionally update ref_${nodeName}_*
end
Template->>Kernel: invoke Gemm_s(..., alpha, beta, transA, transB, offsets, ...)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
Deeploy/Targets/PULPOpen/TopologyOptimizationPasses/Passes.py (2)
181-181: Consider simplifying the exception message.As per Ruff static analysis hint, avoid specifying long messages outside the exception class for better maintainability.
Apply this diff:
- raise ValueError(f"Cannot convert div to integer. Received {div_attr}") + raise ValueError(f"Unsupported div type: {type(div_attr)}")
196-196: Consider using unpacking for list concatenation.As per Ruff hint, use unpacking syntax for more idiomatic Python.
Apply this diff:
- graph.replaceInsertNode(list(conv.inputs) + [mul, add], rqs.outputs, rqsConv) + graph.replaceInsertNode([*list(conv.inputs), mul, add], rqs.outputs, rqsConv)Deeploy/Targets/PULPOpen/Parsers.py (1)
360-364: Refactor to validate before attribute access.Similar to
PULPMatrixVecParser, the variablesM,N, andbatchare computed before validation. If attributes or shapes are unavailable, exceptions will be raised beforesuper().parseNode(node)can validate the node.Apply the same refactoring pattern:
def parseNode(self, node: gs.Node) -> bool: + if not super().parseNode(node): + return False M = node.inputs[0].shape[-1 if node.attrs["transA"] else -2] N = node.inputs[1].shape[-2 if node.attrs["transB"] else -1] batch = math.prod(node.inputs[0].shape[:-2]) - return super().parseNode(node) and M >= 8 and (M % 8) < (N % 8) and batch < 8 + return M >= 8 and (M % 8) < (N % 8) and batch < 8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
CHANGELOG.md(5 hunks)Deeploy/AbstractDataTypes.py(1 hunks)Deeploy/CommonExtensions/DataTypes.py(1 hunks)Deeploy/CommonExtensions/TypeCheckers/SignPropTypeChecker.py(3 hunks)Deeploy/DeeployTypes.py(4 hunks)Deeploy/Targets/Generic/Layers.py(1 hunks)Deeploy/Targets/Generic/Parsers.py(2 hunks)Deeploy/Targets/Generic/Templates/FloatGemmTemplate.py(2 hunks)Deeploy/Targets/Generic/Templates/FloatReduceMeanTemplate.py(1 hunks)Deeploy/Targets/Generic/Templates/GemmTemplate.py(2 hunks)Deeploy/Targets/Generic/TypeCheckers.py(1 hunks)Deeploy/Targets/PULPOpen/Parsers.py(1 hunks)Deeploy/Targets/PULPOpen/Templates/GEMMTemplate.py(2 hunks)Deeploy/Targets/PULPOpen/TileConstraints/ConvTileConstraint.py(3 hunks)Deeploy/Targets/PULPOpen/TileConstraints/DWConvTileConstraint.py(4 hunks)Deeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.py(2 hunks)Deeploy/Targets/PULPOpen/TopologyOptimizationPasses/Passes.py(2 hunks)Deeploy/Targets/Snitch/Bindings.py(1 hunks)Deeploy/Targets/Snitch/Layers.py(1 hunks)Deeploy/Targets/Snitch/Parsers.py(2 hunks)Deeploy/Targets/Snitch/Platform.py(1 hunks)Deeploy/Targets/Snitch/Templates/iNoNormTemplate.py(1 hunks)Deeploy/Targets/Snitch/TypeCheckers.py(1 hunks)DeeployTest/Platforms/Generic/CMakeLists.txt(1 hunks)DeeployTest/testRunner_siracusa_l3dma.py(2 hunks)DeeployTest/testRunner_siracusa_mchandma.py(2 hunks)DeeployTest/testRunner_snitch_dma.py(2 hunks)DeeployTest/testUtils/codeGenerate.py(3 hunks)DeeployTest/testUtils/dmaUtils.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-09T15:43:20.195Z
Learnt from: Xeratec
PR: pulp-platform/Deeploy#105
File: Deeploy/Targets/PULPOpen/TileConstraints/GEMMTileConstraint.py:120-124
Timestamp: 2025-09-09T15:43:20.195Z
Learning: In GEMMTileConstraint.serializeTilingSolution, transpose flags (transA, transB) must be read from operatorRepresentation and used to adjust NSize calculation and matrix offset/shape calculations, following the pattern in FloatGEMMTileConstraint.
Applied to files:
Deeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.pyDeeploy/Targets/PULPOpen/TileConstraints/ConvTileConstraint.pyDeeploy/Targets/PULPOpen/TileConstraints/DWConvTileConstraint.py
📚 Learning: 2025-09-24T12:53:00.168Z
Learnt from: diaconuccalin
PR: pulp-platform/Deeploy#117
File: TargetLibraries/PULPOpen/src/DWConvolution_fp32.c:0-0
Timestamp: 2025-09-24T12:53:00.168Z
Learning: In PULP platform code, log2 is defined in pulp_nn_utils and can be used safely without requiring math.h or causing floating-point build/runtime issues, unlike the standard library log2 function.
Applied to files:
DeeployTest/testUtils/codeGenerate.py
🧬 Code graph analysis (24)
Deeploy/Targets/Snitch/TypeCheckers.py (3)
Deeploy/CommonExtensions/TypeCheckers/SignPropTypeChecker.py (1)
SignPropTypeChecker(15-59)Deeploy/AbstractDataTypes.py (2)
Pointer(311-396)typeWidth(407-408)Deeploy/DeeployTypes.py (2)
VariableBuffer(232-360)inputs(2502-2519)
Deeploy/Targets/PULPOpen/Templates/GEMMTemplate.py (1)
Deeploy/AbstractDataTypes.py (1)
nLevels(194-195)
Deeploy/Targets/Snitch/Bindings.py (1)
Deeploy/Targets/Snitch/TypeCheckers.py (1)
iNoNormChecker(11-25)
Deeploy/Targets/Snitch/Templates/iNoNormTemplate.py (1)
Deeploy/DeeployTypes.py (1)
NodeTemplate(87-229)
Deeploy/Targets/Snitch/Layers.py (1)
Deeploy/DeeployTypes.py (2)
NodeMapper(1659-1815)ONNXLayer(1818-2146)
DeeployTest/testRunner_siracusa_mchandma.py (4)
DeeployTest/testUtils/dmaUtils.py (2)
MemcpyParser(64-78)MemcpyTileConstraint(38-61)Deeploy/AbstractDataTypes.py (1)
PointerClass(544-567)Deeploy/DeeployTypes.py (2)
NodeBinding(1511-1656)NodeTypeChecker(1202-1379)Deeploy/TilingExtension/TilerExtension.py (1)
TilingReadyNodeBindings(1026-1034)
Deeploy/CommonExtensions/TypeCheckers/SignPropTypeChecker.py (3)
Deeploy/DeeployTypes.py (8)
NodeTypeChecker(1202-1379)inputs(2502-2519)VariableBuffer(232-360)typeInferGlobalCtxt(1315-1322)NetworkContext(508-1021)lookup(720-752)ConstantBuffer(393-430)outputs(2521-2538)Deeploy/Targets/Generic/TypeCheckers.py (32)
_inferNumLevels(19-24)_inferNumLevels(42-44)_inferNumLevels(59-61)_inferNumLevels(76-78)_inferNumLevels(93-95)_inferNumLevels(110-112)_inferNumLevels(127-129)_inferNumLevels(144-146)_inferNumLevels(158-160)_inferNumLevels(172-174)_inferNumLevels(186-191)_inferNumLevels(203-205)_inferNumLevels(217-219)_inferNumLevels(234-236)_inferNumLevels(251-253)_inferNumLevels(268-270)_inferSignedness(26-34)_inferSignedness(46-51)_inferSignedness(63-68)_inferSignedness(80-85)_inferSignedness(97-102)_inferSignedness(114-119)_inferSignedness(131-136)_inferSignedness(148-150)_inferSignedness(162-164)_inferSignedness(176-178)_inferSignedness(193-195)_inferSignedness(207-209)_inferSignedness(221-226)_inferSignedness(238-243)_inferSignedness(255-260)_inferSignedness(272-279)Deeploy/AbstractDataTypes.py (10)
checkPromotion(101-118)checkPromotion(157-162)checkPromotion(346-351)checkPromotion(448-455)nLevels(194-195)typeMin(187-191)typeMin(254-255)IntegerImmediate(173-232)fitsNumLevels(231-232)fitsNumLevels(354-355)
Deeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.py (3)
Deeploy/Targets/PULPOpen/TileConstraints/GEMMTileConstraint.py (2)
addGeometricalConstraint(21-60)addGeometricalConstraint(194-231)Deeploy/TilingExtension/TilerModel.py (3)
TilerModel(34-402)addTensorDimToModel(143-157)getTensorDimVar(131-135)Deeploy/DeeployTypes.py (2)
NetworkContext(508-1021)lookup(720-752)
Deeploy/Targets/Generic/TypeCheckers.py (3)
Deeploy/Targets/Snitch/TypeCheckers.py (1)
_inferSignedness(20-25)Deeploy/CommonExtensions/TypeCheckers/SignPropTypeChecker.py (1)
_inferSignedness(23-25)Deeploy/DeeployTypes.py (2)
inputs(2502-2519)VariableBuffer(232-360)
DeeployTest/testUtils/dmaUtils.py (1)
Deeploy/DeeployTypes.py (3)
NetworkContext(508-1021)NodeParser(1024-1199)VariableBuffer(232-360)
Deeploy/Targets/Snitch/Parsers.py (1)
Deeploy/DeeployTypes.py (5)
NetworkContext(508-1021)NodeParser(1024-1199)inputs(2502-2519)outputs(2521-2538)_unpack_const(1142-1154)
DeeployTest/testRunner_snitch_dma.py (4)
DeeployTest/testUtils/dmaUtils.py (5)
MemcpyParser(64-78)MemcpyTileConstraint(38-61)generate_graph(85-96)prepare_deployer_with_custom_tiling(336-355)setup_snitch_deployer(300-333)Deeploy/AbstractDataTypes.py (1)
PointerClass(544-567)Deeploy/DeeployTypes.py (3)
CodeTransformation(2289-2323)NodeBinding(1511-1656)NodeTypeChecker(1202-1379)Deeploy/TilingExtension/TilerExtension.py (1)
TilingReadyNodeBindings(1026-1034)
Deeploy/Targets/PULPOpen/TileConstraints/ConvTileConstraint.py (3)
Deeploy/DeeployTypes.py (1)
lookup(720-752)Deeploy/TilingExtension/TilerModel.py (1)
getTensorDimVar(131-135)Deeploy/TilingExtension/TilingCodegen.py (1)
HyperRectangle(24-35)
Deeploy/Targets/PULPOpen/TopologyOptimizationPasses/Passes.py (1)
Deeploy/CommonExtensions/OptimizationPasses/PassClasses.py (1)
replaceInsertNode(100-108)
Deeploy/Targets/PULPOpen/Parsers.py (1)
Deeploy/DeeployTypes.py (1)
parseNode(1034-1049)
Deeploy/Targets/Generic/Layers.py (1)
Deeploy/DeeployTypes.py (2)
NodeMapper(1659-1815)ONNXLayer(1818-2146)
DeeployTest/testUtils/codeGenerate.py (2)
Deeploy/AbstractDataTypes.py (6)
FloatImmediate(235-308)IntegerImmediate(173-232)typeWidth(407-408)typeName(320-321)typeMin(187-191)typeMin(254-255)Deeploy/DeeployTypes.py (2)
VariableBuffer(232-360)lookup(720-752)
DeeployTest/testRunner_siracusa_l3dma.py (4)
DeeployTest/testUtils/dmaUtils.py (2)
MemcpyParser(64-78)MemcpyTileConstraint(38-61)Deeploy/AbstractDataTypes.py (1)
PointerClass(544-567)Deeploy/DeeployTypes.py (2)
NodeBinding(1511-1656)NodeTypeChecker(1202-1379)Deeploy/TilingExtension/TilerExtension.py (1)
TilingReadyNodeBindings(1026-1034)
Deeploy/Targets/PULPOpen/TileConstraints/DWConvTileConstraint.py (2)
Deeploy/TilingExtension/TilerModel.py (1)
getTensorDimVar(131-135)Deeploy/TilingExtension/TilingCodegen.py (1)
HyperRectangle(24-35)
Deeploy/DeeployTypes.py (1)
Deeploy/AbstractDataTypes.py (5)
nLevels(194-195)checkPromotion(101-118)checkPromotion(157-162)checkPromotion(346-351)checkPromotion(448-455)
Deeploy/CommonExtensions/DataTypes.py (1)
Deeploy/AbstractDataTypes.py (2)
IntegerImmediate(173-232)typeWidth(407-408)
Deeploy/Targets/Generic/Templates/FloatReduceMeanTemplate.py (1)
Deeploy/AbstractDataTypes.py (1)
nLevels(194-195)
Deeploy/Targets/Generic/Parsers.py (3)
Deeploy/DeeployTypes.py (1)
parseNode(1034-1049)Deeploy/Targets/Snitch/Parsers.py (3)
parseNode(16-27)parseNode(48-59)parseNode(83-96)Deeploy/Targets/Neureka/Parsers.py (7)
parseNode(15-35)parseNode(82-96)parseNode(101-107)parseNode(127-137)parseNode(142-147)parseNode(167-177)parseNode(182-187)
Deeploy/Targets/Snitch/Platform.py (2)
Deeploy/Targets/Snitch/Layers.py (1)
iNoNormLayer(12-24)Deeploy/Targets/Snitch/Parsers.py (3)
SnitchGEMMParser(14-43)SnitchRQGEMMParser(46-75)iNoNormParser(78-107)
🪛 Ruff (0.14.2)
Deeploy/AbstractDataTypes.py
222-222: Prefer TypeError exception for invalid type
(TRY004)
222-222: Avoid specifying long messages outside the exception class
(TRY003)
Deeploy/Targets/Snitch/TypeCheckers.py
16-16: Unused method argument: inputs
(ARG002)
17-17: Unused method argument: operatorRepresentation
(ARG002)
21-21: Unused method argument: operatorRepresentation
(ARG002)
Deeploy/Targets/Snitch/Layers.py
20-20: Unused method argument: operatorRepresentation
(ARG002)
21-21: Unused method argument: channels_first
(ARG002)
Deeploy/CommonExtensions/TypeCheckers/SignPropTypeChecker.py
30-30: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
50-50: Loop control variable nLevels overrides iterable it iterates
(B020)
50-50: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
Deeploy/Targets/Generic/TypeCheckers.py
479-479: Unused method argument: inputs
(ARG002)
480-480: Unused method argument: operatorRepresentation
(ARG002)
Deeploy/Targets/Snitch/Parsers.py
101-101: Unused method argument: channels_first
(ARG002)
Deeploy/Targets/PULPOpen/TopologyOptimizationPasses/Passes.py
181-181: Avoid specifying long messages outside the exception class
(TRY003)
196-196: Consider [*list(conv.inputs), mul, add] instead of concatenation
Replace with [*list(conv.inputs), mul, add]
(RUF005)
DeeployTest/testUtils/codeGenerate.py
48-49: Avoid specifying long messages outside the exception class
(TRY003)
Deeploy/DeeployTypes.py
1316-1316: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
1320-1320: Create your own exception
(TRY002)
1320-1320: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (40)
- GitHub Check: siracusa-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: cortexm-models / test-runner-cortexm
- GitHub Check: cortexm-kernels / test-runner-cortexm
- GitHub Check: siracusa-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: cortexm-models / test-runner-cortexm
- GitHub Check: cortexm-kernels / test-runner-cortexm
- GitHub Check: siracusa-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: cortexm-models / test-runner-cortexm
- GitHub Check: cortexm-kernels / test-runner-cortexm
- GitHub Check: siracusa-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: cortexm-models / test-runner-cortexm
- GitHub Check: cortexm-kernels / test-runner-cortexm
- GitHub Check: siracusa-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: cortexm-models / test-runner-cortexm
- GitHub Check: cortexm-kernels / test-runner-cortexm
- GitHub Check: siracusa-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: cortexm-models / test-runner-cortexm
- GitHub Check: cortexm-kernels / test-runner-cortexm
- GitHub Check: siracusa-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: cortexm-models / test-runner-cortexm
- GitHub Check: cortexm-kernels / test-runner-cortexm
- GitHub Check: siracusa-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: cortexm-models / test-runner-cortexm
- GitHub Check: cortexm-kernels / test-runner-cortexm
- GitHub Check: siracusa-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: cortexm-models / test-runner-cortexm
- GitHub Check: cortexm-kernels / test-runner-cortexm
- GitHub Check: siracusa-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: cortexm-models / test-runner-cortexm
- GitHub Check: cortexm-kernels / test-runner-cortexm
🔇 Additional comments (26)
DeeployTest/Platforms/Generic/CMakeLists.txt (1)
11-11: Verify${GENERATED_SOURCE}is consistently defined across calling contexts.The change simplifies the path from
${ProjectId}/../../${GENERATED_SOURCE}to${GENERATED_SOURCE}, which removes coupling to the ProjectId-based directory structure. Since CI checks are passing, the build appears functional. However, this is a structural change that depends on${GENERATED_SOURCE}being properly set in the parent CMake scope.To confirm this change is safe across all test configurations, please verify:
${GENERATED_SOURCE}is defined in the parent CMakeLists or toolchain for all invocations- No other test platforms or configurations rely on the old ProjectId-relative path resolution
- This change is part of a coordinated build system refactor (e.g., in parent CMakeLists files)
If this is part of a broader parser refactor of the build infrastructure, a short note or ADR explaining the refactor would be helpful for future maintainers.
Deeploy/Targets/PULPOpen/TopologyOptimizationPasses/Passes.py (1)
6-6: LGTM!The
mathimport is necessary for thelog2computation in the refactored_merge_conv_rq_fun.Deeploy/AbstractDataTypes.py (1)
209-214: LGTM! Proper numpy scalar handling.The addition of
np.numbersupport correctly converts numpy scalars to Python types and validates integer-valued floats before casting. This enhancement aligns with the broader type system improvements in this PR.Deeploy/Targets/PULPOpen/Templates/GEMMTemplate.py (2)
44-44: LGTM! Enhanced comment for debugging.Adding
nodeNameandnodeOpto the template comment improves code traceability and aligns with similar updates across other templates in this PR.
85-90: LGTM! Explicit None checks improve clarity.Replacing implicit attribute checks with explicit
is not Nonechecks fornLevelsis clearer and more maintainable. This pattern is consistent with similar refactors in FloatReduceMeanTemplate.Deeploy/Targets/Generic/Templates/FloatReduceMeanTemplate.py (1)
21-25: LGTM! Explicit guards improve robustness.Replacing
hasattrchecks with explicitis not Noneguards for both_signedandnLevelsis more defensive and clearer. This pattern is consistent with similar refactors in GEMMTemplate.py.Deeploy/CommonExtensions/DataTypes.py (1)
90-94: LGTM! Essential type correctness fix.Wrapping
sorted()withtuple()ensuresIntegerDataTypescorrectly matches its declared type annotationTuple[Type[IntegerImmediate], ...]. Without this,sorted()returns alist, violating the type contract.DeeployTest/testUtils/codeGenerate.py (4)
5-5: LGTM! Necessary imports for new functionality.The new imports (
math,FloatImmediate,IntegerImmediate) support the centralizedgenerateArrayfunction.Also applies to: 11-11
35-73: LGTM! Well-designed centralization with proper type handling.The
generateArrayfunction effectively centralizes array generation with type-aware literal suffixing and alignment padding. The size assertion prevents silent mismatches, and the handling of float specials (inf/nan) is correct.
90-101: LGTM! Improved input generation with explicit validation.The refactored logic using
generateArrayadds explicit buffer size validation and tiling, ensuring test inputs correctly fill their buffers. The assertion on line 94-95 prevents silent shape mismatches.
111-129: LGTM! Consistent refactor reduces duplication.The refactored
generateTestOutputsHeaderfollows the same pattern as inputs, usinggenerateArrayto eliminate code duplication and improve maintainability.Deeploy/Targets/Snitch/Bindings.py (1)
14-14: LGTM! Completes iNoNorm migration to Snitch.The addition of
iNoNormTemplateandiNoNormCheckerimports from Snitch components completes the migration of iNoNorm functionality from Generic to Snitch, as described in the PR objectives.Also applies to: 18-19, 23-23
Deeploy/Targets/Snitch/Templates/iNoNormTemplate.py (1)
7-10: LGTM! Simplifies template instantiation.Removing the wrapper class and directly instantiating
NodeTemplateeliminates unnecessary indirection without changing functionality. This simplification improves code clarity.Deeploy/Targets/Generic/Layers.py (1)
10-10: Import cleanup for OperatorRepresentation is safe — no external dependencies found.The removal of
OperatorRepresentationfrom the Generic/Layers import is verified as safe. A comprehensive search confirms no modules depend on this file exportingOperatorRepresentation; all consuming code imports the type directly fromDeeploy.DeeployTypes.Deeploy/Targets/Snitch/Parsers.py (1)
78-107: LGTM!The
iNoNormParserimplementation correctly follows theNodeParserpattern. The use ofstrict=Truein thezipcalls at lines 102 and 104 is good practice, ensuring the number of inputs/outputs matches expectations.The static analysis warning about the unused
channels_firstparameter inparseNodeCtxtcan be safely ignored—this parameter is part of the base class signature and should be retained for polymorphism.CHANGELOG.md (3)
7-7: LGTM!The PR reference is correctly added to the changelog.
93-94: LGTM!The changelog entries accurately describe the refactoring changes made in this PR.
108-109: LGTM!The bug fix entries are clearly documented.
Deeploy/Targets/Generic/Templates/FloatGemmTemplate.py (1)
7-43: LGTM!The ref naming scheme change from
${data_out}_${<tensor>}to${nodeName}_${<tensor>}is applied consistently throughout the template. This improves code clarity by properly scoping references to the specific node. The addition oftransAandtransBparameters (lines 24-25) enhances the template's flexibility.DeeployTest/testRunner_siracusa_l3dma.py (1)
78-82: LGTM!The refactoring to generate per-type bindings using a list comprehension (lines 78-81) is a good improvement. This approach ensures proper type checking for each data type (integers and floats) while maintaining code conciseness. The pattern is consistent with similar changes in other test runners across the PR.
Deeploy/Targets/Snitch/Platform.py (3)
14-23: LGTM!The import updates correctly move iNoNorm support from the Generic target to Snitch-specific modules. This change aligns with the PR objective of moving iNoNorm to Snitch because it uses a Snitch kernel.
41-41: LGTM!The
iNoNormMapperis correctly initialized with the Snitch-specific parser and tiling-ready bindings.
56-56: LGTM!The iNoNorm mapping is properly registered in
SnitchMapping, completing the integration of iNoNorm support into the Snitch platform.DeeployTest/testRunner_siracusa_mchandma.py (1)
79-83: LGTM!The refactoring to per-type bindings mirrors the changes in
testRunner_siracusa_l3dma.py, ensuring consistency across DMA test runners. The implementation correctly generates type-safe bindings for all integer and float data types.Deeploy/DeeployTypes.py (2)
1012-1015: Good refactoring for type safety.The refactored
annotateTypemethod now operates on a concrete buffer instance with an explicit type check, which improves clarity and type safety.
1316-1322: Verify the concerns and confirm the code change requires careful consideration.The review comment raises three valid points, all supported by verification:
zip(strict=True)is appropriate — Python 3.10+ is required by the project, and usingstrict=Truecatches length mismatches betweennode.inputsandself.input_types, addressing a real robustness gap.
TypeErroris more appropriate than bareException— The codebase usesTypeError(line 655) andValueError(line 718) for type-related failures, making the bareExceptionat line 1320 a style inconsistency.
ConstantBufferexclusion requires context — The method nametypeInferGlobalCtxtand its position in the type-checking pipeline suggest the design is intentional: only global constant inputs are processed. However, the silent omission ofVariableBufferinstances deserves verification to ensure this is the intended behavior and not an oversight.The suggestions are sound improvements. Ensure the changes align with broader codebase patterns and the method's documented purpose before applying them.
Xeratec
added
Bug
Xeratec
left a comment
Xeratec
left a comment
There was a problem hiding this comment.
Looks good and I only have minor comments.
- There are many commits which fix something. I assume these bugs were not covered b the CI before, right? In this case, would it make sense to add tests?
- I have some local changes with suggested changes, that I could either share the patch or directly push to this PR.
- Improve C tensor naming by renaming all
ref_${data_out}_withref_${nodeName}_ - Prevent empty node names by extending the
_mangleNodeNames()function.
- Improve C tensor naming by renaming all
Let me know what you prefer.
There was a problem hiding this comment.
I agree with these changes, but there are a lot of other places where ref_${data_out}_ is used. I suggest we globally replace it with ref_${nodeName}_ to improve the readability of the generated code. I have locally implemented the change. Let me know if you want the patch or if I should push directly.
There was a problem hiding this comment.
if you've already done it, feel free to push directly to this branch 👍
There was a problem hiding this comment.
in general I agree it would be better to do the audit globally on some of these refactors, but I also have to be reasonable with my changes 😅 so I would rather omit some refactors if it's a strong opinion to do it in a methodical global way because I don't have the will to do it that way now (in the future for sure, once I get annoyed enough with it)
There was a problem hiding this comment.
Also on which fixes do you feel there should be a test?
I would only consider 779dc66 as a candidate for a test. The rest are either compiler warning fixes, or it would require me to get creative with how to test it.
There was a problem hiding this comment.
Okay, this is fine for me. Can you add it? In the meantime, I will implement the other requested changes, and then we should be ready to merge.
…r to parseNode from parseNodeCtxt
…rqs add and mul tensors
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Deeploy/Targets/Generic/Templates/FloatReduceMeanTemplate.py (1)
74-77: Syntax error: missing closing parenthesis in cast.This breaks compilation.
-% if keepdims: -*dummy_${data_out}++ = (${data_out_type.referencedType.typeName}) ((${data_out}_accumulator / ${reduceLength} + ${output_offset}); -% else: -*dummy_${data_out}++ = (${data_out_type.referencedType.typeName}) (${data_out}_accumulator / ${reduceLength}); -% endif +% if keepdims: +*dummy_${data_out}++ = (${data_out_type.referencedType.typeName}) ((${data_out}_accumulator / ${reduceLength}) + ${output_offset}); +% else: +*dummy_${data_out}++ = (${data_out_type.referencedType.typeName}) (${data_out}_accumulator / ${reduceLength}); +% endif
♻️ Duplicate comments (8)
Deeploy/Targets/PULPOpen/TopologyOptimizationPasses/Passes.py (2)
174-176: Replace assertion with explicit validation.The assertion on line 175 can be disabled with Python's
-Oflag, making it unreliable for production validation. Use an explicit check withValueError.Apply this diff:
if isinstance(div_attr, gs.Constant): - assert div_attr.values.size == 1 + if div_attr.values.size != 1: + raise ValueError(f"Expected scalar div but got shape {div_attr.values.shape}") div_attr = div_attr.values.item()
190-191: Usecopy.deepcopyto avoid mutating shared state.The code directly mutates
add.valueswithout copying, which is inconsistent with_merge_gemm_rq_fun(line 229) that usescopy.deepcopybefore modifying values. Direct mutation could cause unexpected side effects ifaddis referenced elsewhere in the graph.Apply this diff:
# Artifically add half the division value as rounding if shift > 0: - add.values += 2**(shift - 1) + add.values = copy.deepcopy(add.values) + 2**(shift - 1)Deeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.py (1)
52-62: Still need right-aligned batch mappingWe’re back to equating batch dims strictly by position. For shapes like
A:(B1,B2,M,K),B:(B2,K,O),Out:(B1,B2,M,O)this forcesB1 == B2, so the solver rejects otherwise valid batched matmuls. We have to right-align (broadcast-style) the batch axes before constraining them. Please restore the right-aligned mapping we discussed earlier.Deeploy/AbstractDataTypes.py (1)
221-222: Use TypeError and a shorter message for unsupported types.Aligns with Ruff TRY004/TRY003 and prior review.
- else: - raise ValueError(f"Unsupported value of type {type(value)} with value {value}") + else: + raise TypeError(f"Unsupported value type {type(value).__name__}")Deeploy/DeeployTypes.py (1)
254-256: Make types Optional to match None initialization.Static checkers will flag bool/int initialized with None.
- self._signed: bool = None - self.nLevels: int = None + self._signed: Optional[bool] = None + self.nLevels: Optional[int] = NoneEnsure
Optionalis imported (it already is in this file).Deeploy/Targets/Generic/TypeCheckers.py (1)
479-481: Fix the return type annotation.The return type annotation specifies
List[int], but the abstract method signature inSignPropTypeCheckerrequiresList[bool]. This inconsistency should be corrected.Apply this diff to fix the return type:
def _inferSignedness(self, inputs: List[VariableBuffer], - operatorRepresentation: OperatorRepresentation) -> List[int]: + operatorRepresentation: OperatorRepresentation) -> List[bool]: return []Deeploy/Targets/Snitch/Layers.py (1)
20-24: Fix the return type annotation.The return type annotation specifies
Tuple[Shape](a tuple containing a singleShape), but the method returns a tuple of two elements: input shapes and output shapes. This should be corrected toTuple[Shape, Shape].Apply this diff to fix the return type:
def computeShapes(self, inputShapes: Shape, outputShapes: Shape, operatorRepresentation: OperatorRepresentation, - channels_first: bool) -> Tuple[Shape]: + channels_first: bool) -> Tuple[Shape, Shape]: # JUNGVI: Broadcast the weights and bias to have as many dimensions as the inputs shape = np.broadcast_shapes(*inputShapes) return ([shape] * len(inputShapes), outputShapes)Deeploy/Targets/Snitch/TypeCheckers.py (1)
16-18: Fix nLevels exponent calculation.
referencedType.typeWidthis already measured in bits, so2**(4 * typeWidth)explodes the level count. For an 8-bit input, this gives 2^32 levels instead of a reasonable value. This will triggerIntegerImmediate.fitsNumLevelswarnings and prevent realistic quantization bounds from propagating.Apply this diff to fix the calculation:
def _inferNumLevels(self, inputs: List[VariableBuffer], operatorRepresentation: OperatorRepresentation) -> List[int]: - return [2**(4 * self.input_types[0].referencedType.typeWidth)] + if inputs[0].nLevels is not None: + return [inputs[0].nLevels] + return [2 ** self.input_types[0].referencedType.typeWidth]
🧹 Nitpick comments (7)
Deeploy/Targets/PULPOpen/TopologyOptimizationPasses/Passes.py (1)
193-202: Consider using unpacking syntax for cleaner concatenation.The list concatenation on line 202 can be modernized using unpacking syntax for better readability.
As per static analysis:
Apply this diff:
-graph.replaceInsertNode(list(conv.inputs) + [mul, add], rqs.outputs, rqsConv) +graph.replaceInsertNode([*list(conv.inputs), mul, add], rqs.outputs, rqsConv)Or more concisely:
-graph.replaceInsertNode(list(conv.inputs) + [mul, add], rqs.outputs, rqsConv) +graph.replaceInsertNode([*conv.inputs, mul, add], rqs.outputs, rqsConv)DeeployTest/testUtils/codeGenerate.py (3)
59-61: Use 64-bit integer literal suffix 'll'/'ull' for portability.On 32-bit targets, 'l' may be 32-bit. Prefer 'll' for 64-bit widths.
- suffix += "l" if refTy.typeWidth >= 64 else "" + if refTy.typeWidth >= 64: + suffix += "ll"And keep 'u' prefix when unsigned, i.e., build as
"ull"or"ll". Ensure order: unsigned first, then 'll'.
65-71: Pad with typed zero for float arrays (optional).Current "0" is fine, but "0f" improves clarity for fp32.
- list_str += ", " + (", ").join("0" for _ in range(paddingElements)) + zero = "0" + (suffix if issubclass(refTy, FloatImmediate) else "") + list_str += ", " + (", ").join(zero for _ in range(paddingElements))
90-101: Input tiling check: good; one nit.Asserts are fine for tests. If you want explicit errors in non-assert runs, consider raising ValueError instead of assert.
Deeploy/DeeployTypes.py (1)
1316-1322: Tighten zip and raise a specific exception with a short message.Use zip(strict=True) and raise ValueError/TypeError instead of bare Exception.
- for tensor, ty in zip(node.inputs, self.input_types): + for tensor, ty in zip(node.inputs, self.input_types, strict=True): buffer = ctxt.lookup(tensor.name) if isinstance(buffer, ConstantBuffer): if not ty.referencedType.checkPromotion(buffer.values): - raise Exception(f"Can't cast {buffer} to {ty}!") + raise TypeError(f"Incompatible constant type for {tensor.name}") ctxt.annotateType(tensor.name, ty)Deeploy/Targets/Generic/Templates/GemmTemplate.py (1)
28-36: Align offset guards with explicit None checks.Using hasattr will compute offsets when attributes exist but are None. Match the new pattern used elsewhere.
- if hasattr(A, "_signed") and hasattr(A, "nLevels"): + if (A._signed is not None) and (A.nLevels is not None): operatorRepresentation['A_offset'] = (A._signed == 0) * int(A.nLevels / 2) - if hasattr(B, "_signed") and hasattr(B, "nLevels"): + if (B._signed is not None) and (B.nLevels is not None): operatorRepresentation['B_offset'] = (B._signed == 0) * int(B.nLevels / 2) - if hasattr(C, "_signed") and hasattr(C, "nLevels"): + if (C._signed is not None) and (C.nLevels is not None): operatorRepresentation['C_offset'] = -(C._signed == 0) * int(C.nLevels / 2) - if hasattr(Y, "_signed") and hasattr(Y, "nLevels"): + if (Y._signed is not None) and (Y.nLevels is not None): operatorRepresentation['Y_offset'] = -(Y._signed == 0) * int(Y.nLevels / 2)Deeploy/CommonExtensions/TypeCheckers/SignPropTypeChecker.py (1)
30-36: Consider adding strict=True to zip for safety.The zip on line 30 doesn't use
strict=True. While the lengths should match by design (inputs and input_types), addingstrict=Truewould catch potential mismatches during development.Apply this diff if you want to add explicit strict checking:
- for tensor, ty in zip(node.inputs, self.input_types): + for tensor, ty in zip(node.inputs, self.input_types, strict=True):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
CHANGELOG.md(5 hunks)Deeploy/AbstractDataTypes.py(1 hunks)Deeploy/CommonExtensions/DataTypes.py(1 hunks)Deeploy/CommonExtensions/TypeCheckers/SignPropTypeChecker.py(3 hunks)Deeploy/DeeployTypes.py(4 hunks)Deeploy/Targets/Generic/Layers.py(1 hunks)Deeploy/Targets/Generic/Parsers.py(2 hunks)Deeploy/Targets/Generic/Templates/FloatGemmTemplate.py(2 hunks)Deeploy/Targets/Generic/Templates/FloatReduceMeanTemplate.py(1 hunks)Deeploy/Targets/Generic/Templates/GemmTemplate.py(2 hunks)Deeploy/Targets/Generic/TypeCheckers.py(1 hunks)Deeploy/Targets/PULPOpen/Parsers.py(1 hunks)Deeploy/Targets/PULPOpen/Templates/GEMMTemplate.py(2 hunks)Deeploy/Targets/PULPOpen/TileConstraints/ConvTileConstraint.py(3 hunks)Deeploy/Targets/PULPOpen/TileConstraints/DWConvTileConstraint.py(4 hunks)Deeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.py(2 hunks)Deeploy/Targets/PULPOpen/TopologyOptimizationPasses/Passes.py(2 hunks)Deeploy/Targets/Snitch/Bindings.py(1 hunks)Deeploy/Targets/Snitch/Layers.py(1 hunks)Deeploy/Targets/Snitch/Parsers.py(2 hunks)Deeploy/Targets/Snitch/Platform.py(1 hunks)Deeploy/Targets/Snitch/Templates/iNoNormTemplate.py(1 hunks)Deeploy/Targets/Snitch/TypeCheckers.py(1 hunks)DeeployTest/Platforms/Generic/CMakeLists.txt(1 hunks)DeeployTest/testRunner_siracusa_l3dma.py(2 hunks)DeeployTest/testRunner_siracusa_mchandma.py(2 hunks)DeeployTest/testRunner_snitch_dma.py(2 hunks)DeeployTest/testUtils/codeGenerate.py(3 hunks)DeeployTest/testUtils/dmaUtils.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- Deeploy/Targets/Generic/Layers.py
- DeeployTest/testRunner_snitch_dma.py
- Deeploy/Targets/Snitch/Bindings.py
- Deeploy/CommonExtensions/DataTypes.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-09T15:43:20.195Z
Learnt from: Xeratec
Repo: pulp-platform/Deeploy PR: 105
File: Deeploy/Targets/PULPOpen/TileConstraints/GEMMTileConstraint.py:120-124
Timestamp: 2025-09-09T15:43:20.195Z
Learning: In GEMMTileConstraint.serializeTilingSolution, transpose flags (transA, transB) must be read from operatorRepresentation and used to adjust NSize calculation and matrix offset/shape calculations, following the pattern in FloatGEMMTileConstraint.
Applied to files:
Deeploy/Targets/PULPOpen/TileConstraints/DWConvTileConstraint.pyDeeploy/Targets/PULPOpen/TileConstraints/ConvTileConstraint.pyDeeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.py
📚 Learning: 2025-09-24T12:53:00.168Z
Learnt from: diaconuccalin
Repo: pulp-platform/Deeploy PR: 117
File: TargetLibraries/PULPOpen/src/DWConvolution_fp32.c:0-0
Timestamp: 2025-09-24T12:53:00.168Z
Learning: In PULP platform code, log2 is defined in pulp_nn_utils and can be used safely without requiring math.h or causing floating-point build/runtime issues, unlike the standard library log2 function.
Applied to files:
DeeployTest/testUtils/codeGenerate.py
🧬 Code graph analysis (20)
Deeploy/Targets/Generic/TypeCheckers.py (3)
Deeploy/Targets/Snitch/TypeCheckers.py (1)
_inferSignedness(20-25)Deeploy/CommonExtensions/TypeCheckers/SignPropTypeChecker.py (1)
_inferSignedness(23-25)Deeploy/DeeployTypes.py (2)
inputs(2502-2519)VariableBuffer(232-360)
Deeploy/Targets/PULPOpen/Templates/GEMMTemplate.py (1)
Deeploy/AbstractDataTypes.py (1)
nLevels(194-195)
Deeploy/Targets/Snitch/Parsers.py (1)
Deeploy/DeeployTypes.py (5)
NetworkContext(508-1021)NodeParser(1024-1199)inputs(2502-2519)outputs(2521-2538)_unpack_const(1142-1154)
Deeploy/CommonExtensions/TypeCheckers/SignPropTypeChecker.py (3)
Deeploy/DeeployTypes.py (7)
NodeTypeChecker(1202-1379)inputs(2502-2519)VariableBuffer(232-360)typeInferGlobalCtxt(1315-1322)NetworkContext(508-1021)lookup(720-752)ConstantBuffer(393-430)Deeploy/Targets/Generic/TypeCheckers.py (32)
_inferNumLevels(19-24)_inferNumLevels(42-44)_inferNumLevels(59-61)_inferNumLevels(76-78)_inferNumLevels(93-95)_inferNumLevels(110-112)_inferNumLevels(127-129)_inferNumLevels(144-146)_inferNumLevels(158-160)_inferNumLevels(172-174)_inferNumLevels(186-191)_inferNumLevels(203-205)_inferNumLevels(217-219)_inferNumLevels(234-236)_inferNumLevels(251-253)_inferNumLevels(268-270)_inferSignedness(26-34)_inferSignedness(46-51)_inferSignedness(63-68)_inferSignedness(80-85)_inferSignedness(97-102)_inferSignedness(114-119)_inferSignedness(131-136)_inferSignedness(148-150)_inferSignedness(162-164)_inferSignedness(176-178)_inferSignedness(193-195)_inferSignedness(207-209)_inferSignedness(221-226)_inferSignedness(238-243)_inferSignedness(255-260)_inferSignedness(272-279)Deeploy/AbstractDataTypes.py (10)
checkPromotion(101-118)checkPromotion(157-162)checkPromotion(346-351)checkPromotion(448-455)nLevels(194-195)typeMin(187-191)typeMin(254-255)IntegerImmediate(173-232)fitsNumLevels(231-232)fitsNumLevels(354-355)
Deeploy/Targets/Generic/Parsers.py (3)
Deeploy/DeeployTypes.py (1)
parseNode(1034-1049)Deeploy/Targets/Snitch/Parsers.py (3)
parseNode(16-27)parseNode(48-59)parseNode(83-96)Deeploy/Targets/Neureka/Parsers.py (7)
parseNode(15-35)parseNode(82-96)parseNode(101-107)parseNode(127-137)parseNode(142-147)parseNode(167-177)parseNode(182-187)
DeeployTest/testRunner_siracusa_l3dma.py (3)
Deeploy/AbstractDataTypes.py (1)
PointerClass(544-567)Deeploy/DeeployTypes.py (3)
CodeTransformation(2289-2323)NodeBinding(1511-1656)NodeTypeChecker(1202-1379)Deeploy/TilingExtension/TilerExtension.py (1)
TilingReadyNodeBindings(1026-1034)
Deeploy/Targets/PULPOpen/TileConstraints/DWConvTileConstraint.py (2)
Deeploy/TilingExtension/TilerModel.py (1)
getTensorDimVar(131-135)Deeploy/TilingExtension/TilingCodegen.py (1)
HyperRectangle(24-35)
Deeploy/Targets/PULPOpen/TileConstraints/ConvTileConstraint.py (3)
Deeploy/DeeployTypes.py (1)
lookup(720-752)Deeploy/TilingExtension/TilerModel.py (1)
getTensorDimVar(131-135)Deeploy/TilingExtension/TilingCodegen.py (1)
HyperRectangle(24-35)
Deeploy/Targets/PULPOpen/Parsers.py (1)
Deeploy/DeeployTypes.py (1)
parseNode(1034-1049)
DeeployTest/testUtils/dmaUtils.py (1)
Deeploy/DeeployTypes.py (4)
NetworkContext(508-1021)NodeParser(1024-1199)NodeTemplate(87-229)VariableBuffer(232-360)
DeeployTest/testUtils/codeGenerate.py (2)
Deeploy/AbstractDataTypes.py (5)
FloatImmediate(235-308)IntegerImmediate(173-232)typeWidth(407-408)typeMin(187-191)typeMin(254-255)Deeploy/DeeployTypes.py (1)
VariableBuffer(232-360)
Deeploy/Targets/Generic/Templates/FloatReduceMeanTemplate.py (1)
Deeploy/AbstractDataTypes.py (1)
nLevels(194-195)
Deeploy/Targets/PULPOpen/TopologyOptimizationPasses/Passes.py (1)
Deeploy/CommonExtensions/OptimizationPasses/PassClasses.py (1)
replaceInsertNode(100-108)
Deeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.py (2)
Deeploy/TilingExtension/TilerModel.py (3)
TilerModel(34-402)addTensorDimToModel(143-157)getTensorDimVar(131-135)Deeploy/DeeployTypes.py (2)
NetworkContext(508-1021)lookup(720-752)
Deeploy/Targets/Snitch/Templates/iNoNormTemplate.py (1)
Deeploy/DeeployTypes.py (1)
NodeTemplate(87-229)
DeeployTest/testRunner_siracusa_mchandma.py (4)
DeeployTest/testUtils/dmaUtils.py (3)
MemcpyParser(64-78)MemcpyTileConstraint(38-61)generate_graph(85-96)Deeploy/AbstractDataTypes.py (1)
PointerClass(544-567)Deeploy/DeeployTypes.py (2)
NodeBinding(1511-1656)NodeTypeChecker(1202-1379)Deeploy/TilingExtension/TilerExtension.py (1)
TilingReadyNodeBindings(1026-1034)
Deeploy/Targets/Snitch/Platform.py (2)
Deeploy/Targets/Snitch/Layers.py (1)
iNoNormLayer(12-24)Deeploy/Targets/Snitch/Parsers.py (1)
iNoNormParser(78-107)
Deeploy/DeeployTypes.py (1)
Deeploy/AbstractDataTypes.py (5)
nLevels(194-195)checkPromotion(101-118)checkPromotion(157-162)checkPromotion(346-351)checkPromotion(448-455)
Deeploy/Targets/Snitch/Layers.py (1)
Deeploy/DeeployTypes.py (2)
NodeMapper(1659-1815)ONNXLayer(1818-2146)
Deeploy/Targets/Snitch/TypeCheckers.py (3)
Deeploy/CommonExtensions/TypeCheckers/SignPropTypeChecker.py (1)
SignPropTypeChecker(15-59)Deeploy/AbstractDataTypes.py (2)
Pointer(311-396)typeWidth(407-408)Deeploy/DeeployTypes.py (2)
VariableBuffer(232-360)inputs(2502-2519)
🪛 Ruff (0.14.2)
Deeploy/Targets/Generic/TypeCheckers.py
479-479: Unused method argument: inputs
(ARG002)
480-480: Unused method argument: operatorRepresentation
(ARG002)
Deeploy/Targets/Snitch/Parsers.py
101-101: Unused method argument: channels_first
(ARG002)
Deeploy/CommonExtensions/TypeCheckers/SignPropTypeChecker.py
30-30: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
50-50: Loop control variable nLevels overrides iterable it iterates
(B020)
50-50: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
DeeployTest/testUtils/codeGenerate.py
48-49: Avoid specifying long messages outside the exception class
(TRY003)
Deeploy/Targets/PULPOpen/TopologyOptimizationPasses/Passes.py
183-183: Avoid specifying long messages outside the exception class
(TRY003)
202-202: Consider [*list(conv.inputs), mul, add] instead of concatenation
Replace with [*list(conv.inputs), mul, add]
(RUF005)
Deeploy/AbstractDataTypes.py
222-222: Prefer TypeError exception for invalid type
(TRY004)
222-222: Avoid specifying long messages outside the exception class
(TRY003)
Deeploy/DeeployTypes.py
1316-1316: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
1320-1320: Create your own exception
(TRY002)
1320-1320: Avoid specifying long messages outside the exception class
(TRY003)
Deeploy/Targets/Snitch/Layers.py
20-20: Unused method argument: operatorRepresentation
(ARG002)
21-21: Unused method argument: channels_first
(ARG002)
Deeploy/Targets/Snitch/TypeCheckers.py
16-16: Unused method argument: inputs
(ARG002)
17-17: Unused method argument: operatorRepresentation
(ARG002)
21-21: Unused method argument: operatorRepresentation
(ARG002)
🔇 Additional comments (16)
DeeployTest/Platforms/Generic/CMakeLists.txt (2)
11-11: Path simplification aligns with PR objectives.The change removes the project-anchored relative path, which aligns with the stated goal of centralizing generated source path handling. This is a straightforward refactor that assumes
${GENERATED_SOURCE}is consistently resolved by the build system.
11-11: Path simplification in Generic is inconsistent with other platforms—verify if intentional or incomplete refactoring.During verification, I found that only 3 of 8 test platforms use
link_directories(): Generic (simplified path), MemPool, and SoftHier (both retaining the older${ProjectId}/../../${GENERATED_SOURCE}pattern). Since the PR shows all checks passing, the simplified path for Generic works in your build system. However, clarify whether this inconsistency is intentional (Generic requires different path handling) or an incomplete refactoring where MemPool and SoftHier should also be simplified. Additionally,${GENERATED_SOURCE}is never explicitly set in any CMake file, confirming it's passed externally—verify this is the expected mechanism.Deeploy/Targets/PULPOpen/TopologyOptimizationPasses/Passes.py (1)
6-6: LGTM!The
mathimport is necessary for thelog2calculation in the refactored_merge_conv_rq_fun.Deeploy/Targets/PULPOpen/Templates/GEMMTemplate.py (1)
85-91: Good: use explicit None checks for A/B/C.nLevels.Safer than hasattr. Please confirm A/B/C._type is always set before alignToContext to avoid attribute errors on referencedType.
CHANGELOG.md (1)
7-7: Changelog entries LGTM.Entries are clear and scoped. No action needed.
Also applies to: 95-97, 111-113
DeeployTest/testUtils/dmaUtils.py (1)
13-15: Import cleanup LGTM.NodeTypeChecker removal here matches per-type bindings elsewhere.
Deeploy/Targets/Generic/Templates/FloatGemmTemplate.py (1)
10-40: LGTM! Consistent variable renaming.The refactor consistently replaces
data_out_*references withnodeName_*throughout the template, improving code readability by aligning variable names with the node identifier. The addition oftransAandtransBparameters is properly integrated into the kernel call.Deeploy/CommonExtensions/TypeCheckers/SignPropTypeChecker.py (1)
5-25: Approve the abstract contract refactor.The introduction of abstract methods
_inferNumLevelsand_inferSignednessproperly establishes a contract for concrete type checkers, improving the extensibility of the type inference system.Deeploy/Targets/Snitch/Templates/iNoNormTemplate.py (1)
7-10: LGTM! Simplified template instantiation.Removing the wrapper subclass and using
NodeTemplatedirectly simplifies the code without changing functionality.Deeploy/Targets/Snitch/Parsers.py (1)
78-107: LGTM! Well-structured parser with proper validation.The
iNoNormParsercorrectly validates node attributes and structure, and properly usesstrict=Truein zip operations to catch length mismatches early. Thelog2Dcomputation and tensor mapping logic are sound.DeeployTest/testRunner_siracusa_mchandma.py (1)
79-86: LGTM! Proper migration to per-type bindings.The refactor correctly replaces the single generic binding with per-datatype bindings, creating one
NodeTypeChecker-based binding for each type inIntegerDataTypes + FloatDataTypes. This aligns with the broader test infrastructure migration mentioned in the PR.DeeployTest/testRunner_siracusa_l3dma.py (2)
9-10: LGTM! Import updates align with per-type binding refactor.The imports correctly add
FloatDataTypes,IntegerDataTypes, andNodeTypeCheckerwhile removing the deprecatedMemcpyTypeChecker, supporting the new per-type binding pattern.Also applies to: 17-18
78-82: LGTM! Per-type binding generation is well-structured.The refactor from a single
MemcpyTypeCheckerbinding to per-typeNodeTypeCheckerbindings is correctly implemented. Each binding properly constructs matching input/output pointer types for its data type, enabling more precise type checking and inference.Deeploy/Targets/Generic/Parsers.py (2)
229-248: LGTM! Defensive validation improves robustness.The refactor adds early validation and prevents partial mutation of
operatorRepresentationon failure. The pattern of checkingsuper().parseNode()first, then validating attribute lengths, and only populating fields on success is a solid defensive coding practice that aligns with similar parsers in the codebase.
276-301: LGTM! Consistent defensive pattern applied.The refactor mirrors the improvements in
MaxPool1DParser, adding early validation and preventing partial state mutation. The validation correctly checks for 2D constraints (4-element pads, 2-element kernel_shape and strides) before populating the operator representation.Deeploy/Targets/Snitch/Platform.py (1)
14-14: LGTM! iNoNorm migration to Snitch platform correctly implemented.The import changes properly migrate
iNoNormLayerandiNoNormParserfrom Generic to Snitch-specific implementations. This aligns with the PR objective of moving iNoNorm to Snitch since it uses Snitch-specific kernels. The Snitch implementations visible in the relevant code snippets confirm they exist and have the appropriate platform-specific logic.Also applies to: 16-16, 22-23
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Deeploy/DeeployTypes.py (1)
254-255: Type annotations inconsistent with initialization values.The type annotations specify
boolandint, but both fields are initialized toNone. This creates a type inconsistency that static type checkers will flag.Apply this diff to use optional types:
- self._signed: bool = None - self.nLevels: int = None + self._signed: Optional[bool] = None + self.nLevels: Optional[int] = NoneEnsure
Optionalis imported at the top of the file (it's already present in line 17).
🧹 Nitpick comments (2)
Deeploy/DeeployTypes.py (2)
1316-1316: Add explicitstrict=Trueto zip for safety.Without the
strictparameter,zip()will silently truncate to the shorter sequence ifnode.inputsandself.input_typeshave mismatched lengths, potentially hiding bugs.Apply this diff:
- for tensor, ty in zip(node.inputs, self.input_types): + for tensor, ty in zip(node.inputs, self.input_types, strict=True):
1319-1320: Consider using a more specific exception type.Using the generic
Exceptionclass makes it harder for calling code to handle specific error cases. Consider usingTypeErrororValueError, or creating a custom exception class for type promotion failures.Example:
if not ty.referencedType.checkPromotion(buffer.values): - raise Exception(f"Can't cast {buffer} to {ty}!") + raise TypeError(f"Can't cast {buffer} to {ty}!")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
CHANGELOG.md(5 hunks)Deeploy/DeeployTypes.py(5 hunks)Deeploy/Targets/CortexM/Templates/GEMMTemplate.py(1 hunks)Deeploy/Targets/Generic/Templates/ConvTemplate.py(2 hunks)Deeploy/Targets/Generic/Templates/ConvTransposeTemplate.py(1 hunks)Deeploy/Targets/Generic/Templates/DWConvTemplate.py(2 hunks)Deeploy/Targets/Generic/Templates/FloatConvTemplate.py(2 hunks)Deeploy/Targets/Generic/Templates/FloatDWConvTemplate.py(1 hunks)Deeploy/Targets/Generic/Templates/FloatMatMulTemplate.py(1 hunks)Deeploy/Targets/Generic/Templates/FloatMaxPoolTemplate.py(2 hunks)Deeploy/Targets/Generic/Templates/MatMulTemplate.py(1 hunks)Deeploy/Targets/Generic/Templates/MaxPoolTemplate.py(1 hunks)Deeploy/Targets/MemPool/Templates/ConvTemplate.py(2 hunks)Deeploy/Targets/MemPool/Templates/DWConvTemplate.py(2 hunks)Deeploy/Targets/MemPool/Templates/GemmTemplate.py(2 hunks)Deeploy/Targets/MemPool/Templates/MatMulTemplate.py(2 hunks)Deeploy/Targets/MemPool/Templates/MaxPoolTemplate.py(1 hunks)Deeploy/Targets/MemPool/Templates/RQGemmTemplate.py(3 hunks)Deeploy/Targets/MemPool/Templates/RQMatMulTemplate.py(4 hunks)Deeploy/Targets/PULPOpen/Parsers.py(1 hunks)Deeploy/Targets/PULPOpen/Templates/FloatConvTemplate.py(2 hunks)Deeploy/Targets/PULPOpen/Templates/FloatGemmTemplate.py(1 hunks)Deeploy/Targets/PULPOpen/Templates/FloatMaxPoolTemplate.py(1 hunks)Deeploy/Targets/PULPOpen/Templates/GEMMTemplate.py(4 hunks)Deeploy/Targets/Snitch/Layers.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-24T12:49:17.889Z
Learnt from: diaconuccalin
Repo: pulp-platform/Deeploy PR: 117
File: Deeploy/Targets/PULPOpen/Templates/FloatConvTemplate.py:100-0
Timestamp: 2025-09-24T12:49:17.889Z
Learning: In Deeploy's PULP FloatConvTemplate.py, the parameter order for PULP_Conv2d_Im2Col_fp*_HWC calls uses X,Y ordering (dim_im_in_x, dim_im_in_y, dim_kernel_x, dim_kernel_y, stride_x, stride_y) which is correct for the implementation, despite appearing different from some other function signatures.
Applied to files:
Deeploy/Targets/MemPool/Templates/DWConvTemplate.pyDeeploy/Targets/Generic/Templates/FloatConvTemplate.pyDeeploy/Targets/Generic/Templates/FloatDWConvTemplate.pyDeeploy/Targets/Generic/Templates/DWConvTemplate.pyDeeploy/Targets/MemPool/Templates/ConvTemplate.pyDeeploy/Targets/PULPOpen/Templates/FloatConvTemplate.py
🧬 Code graph analysis (4)
Deeploy/Targets/Snitch/Layers.py (1)
Deeploy/DeeployTypes.py (2)
NodeMapper(1659-1815)ONNXLayer(1818-2146)
Deeploy/Targets/PULPOpen/Templates/GEMMTemplate.py (1)
Deeploy/AbstractDataTypes.py (1)
nLevels(194-195)
Deeploy/Targets/PULPOpen/Parsers.py (3)
Deeploy/DeeployTypes.py (1)
parseNode(1034-1049)Deeploy/Targets/Snitch/Parsers.py (3)
parseNode(16-27)parseNode(48-59)parseNode(83-96)Deeploy/Targets/Generic/Parsers.py (12)
parseNode(19-27)parseNode(49-58)parseNode(81-94)parseNode(102-112)parseNode(163-169)parseNode(193-207)parseNode(229-248)parseNode(276-301)parseNode(341-359)parseNode(381-393)parseNode(430-442)parseNode(477-481)
Deeploy/DeeployTypes.py (1)
Deeploy/AbstractDataTypes.py (5)
nLevels(194-195)checkPromotion(101-118)checkPromotion(157-162)checkPromotion(346-351)checkPromotion(448-455)
🪛 Ruff (0.14.2)
Deeploy/Targets/Snitch/Layers.py
20-20: Unused method argument: operatorRepresentation
(ARG002)
21-21: Unused method argument: channels_first
(ARG002)
Deeploy/DeeployTypes.py
1316-1316: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
1320-1320: Create your own exception
(TRY002)
1320-1320: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2-wmem / test-runner-siracusa-neureka-tiled
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-neureka-kernels-tiled-doublebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2-wmem / test-runner-siracusa-neureka-tiled
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-neureka-kernels-tiled-doublebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2-wmem / test-runner-siracusa-neureka-tiled
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-neureka-kernels-tiled-doublebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2-wmem / test-runner-siracusa-neureka-tiled
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-neureka-kernels-tiled-doublebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2-wmem / test-runner-siracusa-neureka-tiled
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-neureka-kernels-tiled-doublebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2-wmem / test-runner-siracusa-neureka-tiled
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-neureka-kernels-tiled-doublebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2-wmem / test-runner-siracusa-neureka-tiled
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-neureka-kernels-tiled-doublebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2-wmem / test-runner-siracusa-neureka-tiled
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-neureka-kernels-tiled-doublebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2-wmem / test-runner-siracusa-neureka-tiled
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-neureka-kernels-tiled-doublebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2-wmem / test-runner-siracusa-neureka-tiled
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-neureka-kernels-tiled-doublebuffer-L2 / test-runner-siracusa-neureka-tiled
🔇 Additional comments (21)
Deeploy/Targets/PULPOpen/Templates/FloatConvTemplate.py (1)
40-41: LGTM! Clean and consistent refactoring.The renaming from
ref_${data_out}_*toref_${nodeName}_*improves clarity by aligning variable names with node scope rather than output tensor scope. The refactoring is consistently applied across both templates (reference2DTemplate and reference2DIm2ColTemplate) in variable declarations, kernel invocations, and pointer arithmetic.Also applies to: 45-45, 50-50, 55-56, 63-64, 68-68, 78-78, 86-87
Deeploy/DeeployTypes.py (2)
1012-1015: LGTM - Good defensive programming.The refactor to retrieve and assert the buffer type before annotating is a solid improvement that adds type safety.
3367-3371: LGTM - Good handling of edge case.The addition of empty node name handling prevents potential issues downstream and follows the same pattern as the duplicate name handling above it.
Deeploy/Targets/Snitch/Layers.py (1)
12-24: LGTM - Well-structured implementation.The
iNoNormLayerclass is correctly implemented:
- Properly subclasses
ONNXLayercomputeOps()calculation is clear and well-commentedcomputeShapes()correctly broadcasts input shapes to a common shape- The return type annotation
Tuple[Shape, Shape]is correct (contrary to what the past review comment suggested)The static analysis warnings about unused parameters in
computeShapesare false positives—these parameters are required to match the parent class method signature.Deeploy/Targets/MemPool/Templates/RQMatMulTemplate.py (3)
115-119: LGTM! Clear naming improvement.The refactoring from
ref_${data_out}_*toref_${nodeName}_*improves semantic clarity. UsingnodeNameas the prefix is more meaningful and reduces confusion. Note thatmulandaddappropriately retain their original naming since they're context lookups rather than node-specific buffers.
122-174: All function call branches updated consistently.All three conditional paths (unrolled, offset_unrolled, and standard variants) correctly reference the renamed variables. The refactoring is applied consistently across all code branches.
176-178: Pointer arithmetic and naming convention are consistent across all MemPool templates.Verification complete across RQMatMulTemplate, MatMulTemplate, RQGemmTemplate, and GemmTemplate. All templates consistently use
ref_${nodeName}_${VAR}pattern for pointer initialization and arithmetic operations in batch loops. The pointer advances are correctly computed: A by M×N, B by N×O, and data_out by M×O. No naming inconsistencies or missed updates detected.Deeploy/Targets/PULPOpen/Parsers.py (1)
363-370: Past review feedback has been properly addressed.The redundant
super().parseNode(node)call has been correctly removed. The implementation follows the same clean pattern asPULPMatrixVecParserwith appropriate dimension filtering logic for tall GEMM operations.Deeploy/Targets/Generic/Templates/MatMulTemplate.py (1)
37-54: LGTM! Consistent template reference renaming.The refactoring from
data_out-prefixed tonodeName-prefixed references is applied consistently across declarations, function arguments, and pointer arithmetic updates. This improves code clarity by using a more semantically meaningful prefix.Deeploy/Targets/PULPOpen/Templates/FloatGemmTemplate.py (1)
9-30: LGTM! Consistent refactoring with added parameters.The template references have been consistently renamed from
data_out-prefixed tonodeName-prefixed across all declarations, kernel arguments, and accumulation updates. The addition oftransAandtransBparameters aligns with standard GEMM interfaces.Deeploy/Targets/MemPool/Templates/ConvTemplate.py (2)
40-53: LGTM! Consistent renaming in 1D Conv template.The
nodeName-prefixed reference scheme is consistently applied throughout the 1D convolution template, covering declarations, kernel arguments, and batch offset updates.
66-79: LGTM! Consistent renaming in 2D Conv template.The
nodeName-prefixed reference scheme is consistently applied throughout the 2D convolution template, matching the pattern used in the 1D variant.Deeploy/Targets/MemPool/Templates/GemmTemplate.py (1)
114-143: LGTM! Comprehensive GEMM template refactoring.The template consistently applies the
nodeName-prefixed naming scheme and extends the kernel call with standard GEMM parameters (alpha, beta, transA, transB, offsets). The batch loop correctly increments A, B, and data_out while keeping C constant, which is appropriate when C serves as a reusable bias term.Deeploy/Targets/Generic/Templates/FloatDWConvTemplate.py (1)
14-28: LGTM! Consistent renaming in depthwise convolution template.The
nodeName-prefixed reference scheme is consistently applied across declarations, kernel arguments, and batch offset updates in the 2D depthwise convolution template.Deeploy/Targets/Generic/Templates/ConvTemplate.py (2)
39-50: LGTM! Consistent renaming in 1D Conv template.The
nodeName-prefixed reference scheme is consistently applied throughout the 1D convolution template.
63-74: LGTM! Consistent renaming in 2D Conv template.The
nodeName-prefixed reference scheme is consistently applied throughout the 2D convolution template, maintaining consistency with the 1D variant.Deeploy/Targets/Generic/Templates/ConvTransposeTemplate.py (1)
15-28: LGTM! Consistent renaming in transposed convolution template.The
nodeName-prefixed reference scheme is consistently applied across declarations, kernel arguments, and batch offset updates in the 1D transposed convolution template.Deeploy/Targets/Generic/Templates/FloatMatMulTemplate.py (1)
10-26: LGTM! Consistent renaming in float MatMul template.The
nodeName-prefixed reference scheme is consistently applied throughout the template, covering declarations, function arguments, and accumulation updates. This matches the refactoring pattern applied to the integer MatMul template.Deeploy/Targets/PULPOpen/Templates/GEMMTemplate.py (3)
44-66: LGTM! Consistent variable renaming improves clarity.The renaming from
data_out-prefixed tonodeName-prefixed references is semantically correct and makes the generated code more understandable. All references are updated consistently throughout the template, including the function call and pointer arithmetic.
85-90: LGTM! Explicit None checks are more Pythonic.Replacing
hasattr()with explicitis not Nonechecks is an improvement:
- More explicit about the intent (checking for None values vs attribute existence)
- Will fail fast with AttributeError if the attribute is missing, aiding debugging
- More idiomatic Python when attributes are expected to exist
96-118: LGTM! MatMul template consistently updated.The same nodeName-based renaming pattern is correctly applied here. All variable declarations, function call parameters, and pointer arithmetic are consistently updated.
|
@Xeratec I've checked your changes and it all makes sense. In the current implementation of the def computeShapes(self, inputShapes: Shape, outputShapes: Shape, operatorRepresentation,
channels_first) -> Tuple[Shape, Shape]:
if channels_first:
inputShapes[2] = [outputShapes[0][1]] # Channels out dimension of Kernel
inputShapes[3] = [outputShapes[0][1]] # Channels out dimension of Kernel
else:
inputShapes[2] = [outputShapes[0][-1]] # Channels out dimension of Kernel
inputShapes[3] = [outputShapes[0][-1]] # Channels out dimension of Kernel
return (inputShapes, outputShapes)This makes commit 779dc66 not a fix at the moment, but more of a refactor. It is a fix in my draft PR #132 since I've changed how I suggest we either leave this here and confirm it's correctness just by looking at the changes, i.e. no extra test for it, or we move the commit to the draft PR branch where it actually fixes something. Let me know what you would like me to do. |
2 participants
This PR is a collection of refactors and fixes I've done while working on the parser refactor.
Each commit holds a small change so best reviewed commit-by-commit.
Changed
${data_out}_${<tensor>}to${nodeName}_${<tensor>}Fixed
PR Merge Checklist
develcommit and pointing todevel.CHANGELOG.mdfile has been updated.