Add OperatorDescriptor #118
Conversation
lukamac
force-pushed
the
parser-refactor-operator-descriptors
branch
2 times, most recently
from
b3c01ee to
8b74b53
Compare
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughIntroduce an OperatorDescriptor system and default registry; add IoDesc/AttrDesc classes; thread operatorDescriptors through NetworkContainer/NetworkDeployer and many target deployers; add a large OperatorDescriptor module; refactor parsers, GEMM templates (cast trans flags), reshape/reducemean bindings, and test wiring. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Deployer
participant Container
participant Binder
participant Descriptor
Caller ->> Deployer: __init__(graph,..., operatorDescriptors)
Deployer ->> Container: __init__(..., operatorDescriptors)
Container ->> Container: store operatorDescriptors
Caller ->> Deployer: run binding
Deployer ->> Binder: _bindLayers()
loop per node
Binder ->> Container: operatorDescriptors[node.op]
Container -->> Binder: desc
Binder ->> Descriptor: desc.canonicalize(node, opset)
Descriptor -->> Binder: updated node attrs/inputs
Binder ->> Descriptor: desc.check(node)
alt valid
Binder ->> node: proceed to bind layer
else invalid
Binder -->> Deployer: assert/raise
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CHANGELOG.md (1)
4-405: CHANGELOG update is incomplete and blocking this PR.The PR checklist explicitly notes "CHANGELOG update remain pending." This is a known blocker. The provided changes show only formatting/marker adjustments to existing entries (lines 176–178, 317) but do not document the major OperatorDescriptor refactor:
- Missing from "Added" section: OperatorDescriptor, IoDescriptor, AttrDescriptor class descriptions; node canonicalization and checking functionality.
- Missing from "Changed" section: Updates to multiple deployer classes across targets to accept and propagate descriptor mappings.
Per your PR objectives, this refactor is a substantial architectural change. The CHANGELOG must document it before merge.
Suggested CHANGELOG entries:
### Added + - OperatorDescriptor system: target-agnostic operator information with IoDescriptor and AttrDescriptor + - Node canonicalization and checking for operators (Parser refactor)### Changed + - All deployer classes across multiple targets updated to accept operatorDescriptors during initializationDo you want me to help you generate comprehensive CHANGELOG entries that document the full scope of this refactor?
🧹 Nitpick comments (8)
Deeploy/Targets/Snitch/Parsers.py (1)
21-21: LGTM! Simplified condition maintains equivalent logic.The refactored condition
if self.operatorRepresentation['transA']:is more idiomatic and maintains the same semantics: reject whentransAis non-zero (truthy), accept when it's zero (falsy).Deeploy/Targets/Generic/TypeCheckers.py (1)
188-189: Consider renaming variable for clarity.The extraction of the dimension calculation into variable
Oimproves readability, but the single-letter nameOcan be visually confused with0(zero). Consider a more descriptive name likeoutput_dimorsecond_dim.Apply this diff:
- O = inputs[0].shape[-1] if not operatorRepresentation['transA'] else inputs[0].shape[-2] - return [2**((self.input_types[0].referencedType.typeWidth) * 2) * O] + output_dim = inputs[0].shape[-1] if not operatorRepresentation['transA'] else inputs[0].shape[-2] + return [2**((self.input_types[0].referencedType.typeWidth) * 2) * output_dim]Deeploy/Targets/Generic/TopologyOptimizationPasses/Passes.py (1)
356-399: LGTM! Refactor improves clarity with early returns.The refactored
_extract_padding_fun_convfunction is clearer with explicit early returns for edge cases (no pads attribute, all-zero padding) and simplified padding computation logic. The core functionality appears preserved.Optional: Consider static analysis suggestions.
Two minor improvements suggested by static analysis:
- Line 357: Prefer
next(iter(match.nodes_map.values()))over single-element slice for better intent clarity.- Line 378: Add explicit
strict=parameter tozip()to catch length mismatches.- conv = list(match.nodes_map.values())[0] + conv = next(iter(match.nodes_map.values()))- for dim, begin, end in zip(convShape, pads[:len(pads) // 2], pads[len(pads) // 2:]): + for dim, begin, end in zip(convShape, pads[:len(pads) // 2], pads[len(pads) // 2:], strict=True):Deeploy/Targets/Generic/Parsers.py (1)
1658-1707: Consider renaming variable 'O' for clarity.The refactor to cache buffer lookups (
buffA,buffB,buffOut) and derive dimensions locally improves readability. However, the variable nameOat lines 1677 and 1679 is flagged as ambiguous by static analysis (easily confused with zero).Consider renaming for clarity:
if self.operatorRepresentation['transB']: - O, N_B = buffB.shape[-2:] + N_B_cols, N_B = buffB.shape[-2:] else: - N_B, O = buffB.shape[-2:] + N_B, N_B_cols = buffB.shape[-2:] # Store the matrix dimensions in the operator representation self.operatorRepresentation['M'] = M self.operatorRepresentation['N'] = N_A - self.operatorRepresentation['O'] = O + self.operatorRepresentation['O'] = N_B_colsDeeploy/DeeployTypes.py (3)
1088-1119: Prefer logger over prints and fix membership test styleUse the module logger and
not inidiom.Apply:
- if not self.inputDescriptor.checkTensors(node.inputs): - # TODO: Change to logging - print(f"[ERROR OP {node.op}] Invalid input tensors: {[t.name for t in node.inputs]}") + if not self.inputDescriptor.checkTensors(node.inputs): + log.error(f"[ERROR OP {node.op}] Invalid input tensors: {[t.name for t in node.inputs]}") valid = False @@ - if not self.outputDescriptor.checkTensors(node.outputs): - # TODO: Change to logging - print(f"[ERROR OP {node.op}] Invalid output tensors: {[t.name for t in node.outputs]}") + if not self.outputDescriptor.checkTensors(node.outputs): + log.error(f"[ERROR OP {node.op}] Invalid output tensors: {[t.name for t in node.outputs]}") valid = False @@ - if attrDesc.default is None and not attrDesc.name in node.attrs: - # TODO: Change to logging - print(f"[ERROR OP {node.op}] Missing attribute {attrDesc.name}") + if attrDesc.default is None and attrDesc.name not in node.attrs: + log.error(f"[ERROR OP {node.op}] Missing attribute {attrDesc.name}") valid = False
2583-2609: Ensure operatorDescriptors is provided and coherentAdd a guard so deployments fail early if descriptors are missing or incomplete.
For example:
@@ - operatorDescriptors: Dict[str, OperatorDescriptor], + operatorDescriptors: Dict[str, OperatorDescriptor], @@ - self.operatorDescriptors = operatorDescriptors + assert operatorDescriptors, "operatorDescriptors must be a non-empty mapping" + self.operatorDescriptors = operatorDescriptorsConsider validating that all ops in the graph have descriptors here to fail fast, not during binding.
2738-2744: Handle canonicalize failures explicitly and guard parsing
canonicalize()returns bool and may fail; currently ignored. Also, given canonicalization may alter attrs/inputs, add defensive try/except with clear logs.Apply:
- desc = self.operatorDescriptors[node.op] - desc.canonicalize(node, self.graph.opset) - assert desc.check(node), \ + desc = self.operatorDescriptors[node.op] + ok = False + try: + ok = desc.canonicalize(node, self.graph.opset) + except Exception as e: + log.error(f"[ERROR] Canonicalization failed for node {node.name} ({node.op}): {e}") + ok = False + assert ok and desc.check(node), \ f"[ERROR] Node {node.name} is not a valid instance of {node.op} operator"Note: The CI KeyError ('axes') indicates canonicalization currently removes attrs required by existing parsers. See fix in OperatorDescriptor.attrToTensor to keep attrs until parsers are migrated.
Deeploy/OperatorDescriptor.py (1)
373-388: Use logger instead of print warnings in canonicalizePrefer
log.warningand consistent error handling. Optional polish.Example:
- print("[WARNING] Requantized operator cannot have n_levels_out and n_levels in its attributes") + log.warning("Requantized operator cannot have both n_levels_out and n_levels")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
CHANGELOG.md(2 hunks)Deeploy/CommonExtensions/NetworkDeployers/SignPropDeployer.py(2 hunks)Deeploy/DeeployTypes.py(7 hunks)Deeploy/EngineExtension/NetworkDeployers/EngineColoringDeployer.py(2 hunks)Deeploy/MemoryLevelExtension/NetworkDeployers/MemoryLevelDeployer.py(3 hunks)Deeploy/OperatorDescriptor.py(1 hunks)Deeploy/Targets/Chimera/Deployer.py(3 hunks)Deeploy/Targets/CortexM/Deployer.py(3 hunks)Deeploy/Targets/Generic/Bindings.py(2 hunks)Deeploy/Targets/Generic/Deployer.py(3 hunks)Deeploy/Targets/Generic/Parsers.py(12 hunks)Deeploy/Targets/Generic/Templates/FloatGemmTemplate.py(1 hunks)Deeploy/Targets/Generic/Templates/GemmTemplate.py(1 hunks)Deeploy/Targets/Generic/TopologyOptimizationPasses/Passes.py(1 hunks)Deeploy/Targets/Generic/TypeCheckers.py(1 hunks)Deeploy/Targets/MemPool/Deployer.py(2 hunks)Deeploy/Targets/MemPool/Templates/GemmTemplate.py(1 hunks)Deeploy/Targets/MemPool/Templates/RQGemmTemplate.py(2 hunks)Deeploy/Targets/MemPool/TopologyOptimizationPasses/Passes.py(1 hunks)Deeploy/Targets/Neureka/Deployer.py(2 hunks)Deeploy/Targets/Neureka/Parsers.py(2 hunks)Deeploy/Targets/PULPOpen/Deployer.py(3 hunks)Deeploy/Targets/PULPOpen/Parsers.py(2 hunks)Deeploy/Targets/PULPOpen/Templates/FloatGemmTemplate.py(1 hunks)Deeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.py(1 hunks)Deeploy/Targets/Snitch/Deployer.py(3 hunks)Deeploy/Targets/Snitch/Parsers.py(2 hunks)Deeploy/Targets/SoftHier/Deployer.py(2 hunks)DeeployTest/testMemoryLevelExtension.py(5 hunks)DeeployTest/testUtils/dmaUtils.py(4 hunks)DeeployTest/testUtils/platformMapping.py(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (20)
Deeploy/Targets/Snitch/Deployer.py (1)
Deeploy/DeeployTypes.py (2)
OperatorDescriptor(1083-1169)TopologyOptimizer(2326-2355)
Deeploy/Targets/Generic/Parsers.py (1)
Deeploy/DeeployTypes.py (2)
lookup(720-752)VariableBuffer(232-360)
Deeploy/Targets/Chimera/Deployer.py (1)
Deeploy/DeeployTypes.py (1)
OperatorDescriptor(1083-1169)
Deeploy/MemoryLevelExtension/NetworkDeployers/MemoryLevelDeployer.py (1)
Deeploy/DeeployTypes.py (5)
NetworkContext(508-1020)NetworkDeployer(3335-3707)OperatorDescriptor(1083-1169)TopologyOptimizer(2326-2355)VariableBuffer(232-360)
Deeploy/Targets/PULPOpen/Deployer.py (1)
Deeploy/DeeployTypes.py (2)
OperatorDescriptor(1083-1169)VariableBuffer(232-360)
Deeploy/Targets/CortexM/Deployer.py (1)
Deeploy/DeeployTypes.py (3)
DeploymentPlatform(2528-2571)OperatorDescriptor(1083-1169)TopologyOptimizer(2326-2355)
Deeploy/Targets/Generic/Deployer.py (1)
Deeploy/DeeployTypes.py (2)
OperatorDescriptor(1083-1169)TopologyOptimizer(2326-2355)
DeeployTest/testUtils/platformMapping.py (1)
Deeploy/DeeployTypes.py (4)
DeploymentPlatform(2528-2571)NetworkDeployer(3335-3707)OperatorDescriptor(1083-1169)TopologyOptimizer(2326-2355)
Deeploy/CommonExtensions/NetworkDeployers/SignPropDeployer.py (1)
Deeploy/DeeployTypes.py (4)
DeploymentPlatform(2528-2571)NetworkDeployer(3335-3707)OperatorDescriptor(1083-1169)TopologyOptimizer(2326-2355)
Deeploy/Targets/MemPool/Deployer.py (1)
Deeploy/DeeployTypes.py (3)
DeploymentPlatform(2528-2571)OperatorDescriptor(1083-1169)TopologyOptimizer(2326-2355)
Deeploy/Targets/Generic/Bindings.py (3)
Deeploy/CommonExtensions/DataTypes.py (1)
int64_t(30-33)Deeploy/Targets/Generic/TypeCheckers.py (1)
ReduceMeanChecker(308-322)Deeploy/AbstractDataTypes.py (1)
PointerClass(536-559)
Deeploy/EngineExtension/NetworkDeployers/EngineColoringDeployer.py (1)
Deeploy/DeeployTypes.py (4)
DeploymentPlatform(2528-2571)NetworkDeployer(3335-3707)OperatorDescriptor(1083-1169)TopologyOptimizer(2326-2355)
Deeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.py (1)
Deeploy/TilingExtension/TilerModel.py (1)
getTensorDimVar(131-135)
DeeployTest/testUtils/dmaUtils.py (1)
Deeploy/DeeployTypes.py (5)
IoDesc(1023-1038)NetworkContext(508-1020)NetworkDeployer(3335-3707)OperatorDescriptor(1083-1169)VariableBuffer(232-360)
DeeployTest/testMemoryLevelExtension.py (1)
Deeploy/DeeployTypes.py (1)
OperatorDescriptor(1083-1169)
Deeploy/DeeployTypes.py (1)
Deeploy/OperatorDescriptor.py (4)
canonicalize(91-98)canonicalize(183-187)canonicalize(377-387)canonicalize(664-679)
Deeploy/Targets/Neureka/Deployer.py (1)
Deeploy/DeeployTypes.py (3)
DeploymentPlatform(2528-2571)OperatorDescriptor(1083-1169)TopologyOptimizer(2326-2355)
Deeploy/Targets/SoftHier/Deployer.py (1)
Deeploy/DeeployTypes.py (3)
DeploymentPlatform(2528-2571)OperatorDescriptor(1083-1169)TopologyOptimizer(2326-2355)
Deeploy/Targets/Generic/TypeCheckers.py (1)
Deeploy/AbstractDataTypes.py (1)
typeWidth(399-400)
Deeploy/OperatorDescriptor.py (1)
Deeploy/DeeployTypes.py (6)
AttrDesc(1055-1079)IoDesc(1023-1038)OperatorDescriptor(1083-1169)VariadicIoDesc(1041-1051)inputs(2656-2673)canonicalize(1121-1132)
🪛 GitHub Actions: CI • Generic
Deeploy/DeeployTypes.py
[error] 2799-2799: KeyError: 'axes' encountered during parsing. The attribute 'axes' is missing from node.attrs.
🪛 Ruff (0.14.1)
Deeploy/Targets/Generic/Parsers.py
1677-1677: Ambiguous variable name: O
(E741)
1679-1679: Ambiguous variable name: O
(E741)
Deeploy/MemoryLevelExtension/NetworkDeployers/MemoryLevelDeployer.py
120-120: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
164-164: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
165-165: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
Deeploy/CommonExtensions/NetworkDeployers/SignPropDeployer.py
26-26: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
Deeploy/Targets/MemPool/Deployer.py
30-30: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
Deeploy/Targets/Generic/TopologyOptimizationPasses/Passes.py
357-357: Prefer next(iter(match.nodes_map.values())) over single element slice
Replace with next(iter(match.nodes_map.values()))
(RUF015)
378-378: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
Deeploy/DeeployTypes.py
1025-1025: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
1114-1114: Test for membership should be not in
Convert to not in
(E713)
1131-1131: Avoid specifying long messages outside the exception class
(TRY003)
1157-1157: Undefined name itertools
(F821)
Deeploy/Targets/Neureka/Deployer.py
30-30: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
Deeploy/Targets/SoftHier/Deployer.py
26-26: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
Deeploy/Targets/Generic/TypeCheckers.py
188-188: Ambiguous variable name: O
(E741)
Deeploy/OperatorDescriptor.py
23-23: Avoid specifying long messages outside the exception class
(TRY003)
57-57: Do not use bare except
(E722)
⏰ 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). (150)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-neureka-models-tiled-doublebuffer-L3-wmem (microLlama/microLlama1, 10000, 8, true, L3, t... / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 10000, 8, L3) / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (Transformer, 15000, 8, L3) / test-runner-siracusa-neureka-tiled (15000)
- GitHub Check: deeploy-test-dmas
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNetv2, 60000, 16000, 12000, 8000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (testTrainCCT/CCT1_Classifier_Training/CCT_1_16_16_8, 64000... / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/ImageClassification, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/AnomalyDetection, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-neureka-models-tiled-doublebuffer-L3-wmem (microLlama/microLlama1, 10000, 8, true, L3, t... / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 10000, 8, L3) / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (Transformer, 15000, 8, L3) / test-runner-siracusa-neureka-tiled (15000)
- GitHub Check: deeploy-test-dmas
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNetv2, 60000, 16000, 12000, 8000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (testTrainCCT/CCT1_Classifier_Training/CCT_1_16_16_8, 64000... / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/ImageClassification, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/AnomalyDetection, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-neureka-models-tiled-doublebuffer-L3-wmem (microLlama/microLlama1, 10000, 8, true, L3, t... / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 10000, 8, L3) / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (Transformer, 15000, 8, L3) / test-runner-siracusa-neureka-tiled (15000)
- GitHub Check: deeploy-test-dmas
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNetv2, 60000, 16000, 12000, 8000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (testTrainCCT/CCT1_Classifier_Training/CCT_1_16_16_8, 64000... / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/ImageClassification, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/AnomalyDetection, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-neureka-models-tiled-doublebuffer-L3-wmem (microLlama/microLlama1, 10000, 8, true, L3, t... / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 10000, 8, L3) / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (Transformer, 15000, 8, L3) / test-runner-siracusa-neureka-tiled (15000)
- GitHub Check: deeploy-test-dmas
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNetv2, 60000, 16000, 12000, 8000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (testTrainCCT/CCT1_Classifier_Training/CCT_1_16_16_8, 64000... / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/ImageClassification, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/AnomalyDetection, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-neureka-models-tiled-doublebuffer-L3-wmem (microLlama/microLlama1, 10000, 8, true, L3, t... / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 10000, 8, L3) / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (Transformer, 15000, 8, L3) / test-runner-siracusa-neureka-tiled (15000)
- GitHub Check: deeploy-test-dmas
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNetv2, 60000, 16000, 12000, 8000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (testTrainCCT/CCT1_Classifier_Training/CCT_1_16_16_8, 64000... / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/ImageClassification, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/AnomalyDetection, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-neureka-models-tiled-doublebuffer-L3-wmem (microLlama/microLlama1, 10000, 8, true, L3, t... / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 10000, 8, L3) / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (Transformer, 15000, 8, L3) / test-runner-siracusa-neureka-tiled (15000)
- GitHub Check: deeploy-test-dmas
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNetv2, 60000, 16000, 12000, 8000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (testTrainCCT/CCT1_Classifier_Training/CCT_1_16_16_8, 64000... / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/ImageClassification, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/AnomalyDetection, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-neureka-models-tiled-doublebuffer-L3-wmem (microLlama/microLlama1, 10000, 8, true, L3, t... / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 10000, 8, L3) / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (Transformer, 15000, 8, L3) / test-runner-siracusa-neureka-tiled (15000)
- GitHub Check: deeploy-test-dmas
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNetv2, 60000, 16000, 12000, 8000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (testTrainCCT/CCT1_Classifier_Training/CCT_1_16_16_8, 64000... / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/ImageClassification, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/AnomalyDetection, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-neureka-models-tiled-doublebuffer-L3-wmem (microLlama/microLlama1, 10000, 8, true, L3, t... / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 10000, 8, L3) / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (Transformer, 15000, 8, L3) / test-runner-siracusa-neureka-tiled (15000)
- GitHub Check: deeploy-test-dmas
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNetv2, 60000, 16000, 12000, 8000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (testTrainCCT/CCT1_Classifier_Training/CCT_1_16_16_8, 64000... / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/ImageClassification, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/AnomalyDetection, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-neureka-models-tiled-doublebuffer-L3-wmem (microLlama/microLlama1, 10000, 8, true, L3, t... / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 10000, 8, L3) / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (Transformer, 15000, 8, L3) / test-runner-siracusa-neureka-tiled (15000)
- GitHub Check: deeploy-test-dmas
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNetv2, 60000, 16000, 12000, 8000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (testTrainCCT/CCT1_Classifier_Training/CCT_1_16_16_8, 64000... / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/ImageClassification, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/AnomalyDetection, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-neureka-models-tiled-doublebuffer-L3-wmem (microLlama/microLlama1, 10000, 8, true, L3, t... / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 10000, 8, L3) / test-runner-siracusa-neureka-tiled (10000)
- GitHub Check: siracusa-neureka-models-tiled-singlebuffer-L3 (Transformer, 15000, 8, L3) / test-runner-siracusa-neureka-tiled (15000)
- GitHub Check: deeploy-test-dmas
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNetv2, 60000, 16000, 12000, 8000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (testTrainCCT/CCT1_Classifier_Training/CCT_1_16_16_8, 64000... / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/ImageClassification, 64000, 8) / test-runner-siracusa-tiled (64000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8_parallel, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (microLlama/microLlama8, 60000, 10000, 5000, 8) / test-runner-siracusa-tiled (5000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L2 (MLPerf/AnomalyDetection, 64000, 8) / test-runner-siracusa-tiled (64000)
🔇 Additional comments (39)
Deeploy/Targets/MemPool/Templates/RQGemmTemplate.py (1)
148-149: LGTM! Integer casting ensures type safety.The explicit
int()casting oftransAandtransBensures these flags are passed as integers to the kernel functions, which aligns with similar changes across other GEMM template variants in this PR.Also applies to: 173-174
Deeploy/Targets/Generic/Templates/GemmTemplate.py (1)
59-60: LGTM! Consistent integer casting for transposition flags.The explicit
int()casting aligns with the pattern applied across all GEMM template variants in this PR, ensuring type consistency in generated kernel calls.Deeploy/Targets/MemPool/TopologyOptimizationPasses/Passes.py (1)
292-292: LGTM! Type correction for numeric attribute.Changing
keepdimsfrom string"0"to integer0ensures proper typing for the ReduceSum operation attribute, consistent with the descriptor-based attribute normalization efforts in this PR.Deeploy/Targets/MemPool/Templates/GemmTemplate.py (1)
130-131: LGTM! Consistent with other GEMM template updates.The integer casting of transposition flags matches the pattern applied uniformly across GEMM templates in this PR.
Deeploy/Targets/Snitch/Parsers.py (1)
51-51: LGTM! Consistent simplification with SnitchGEMMParser.Same logic improvement as applied in SnitchGEMMParser.parseNode.
Deeploy/Targets/Generic/Templates/FloatGemmTemplate.py (1)
24-25: LGTM! Consistent integer casting across GEMM templates.The explicit
int()casting ensures transposition flags are passed as integers, consistent with the pattern applied throughout all GEMM template variants.DeeployTest/testMemoryLevelExtension.py (2)
21-21: LGTM! Proper integration of operator descriptors.The import of
defaultOperatorDescriptorsis necessary for wiring the new descriptor system into the test infrastructure.
87-87: LGTM! Consistent descriptor propagation across all test deployers.All four MemoryLevelAwareSignPropDeployer instantiations (CMSIS, MemPool, Generic, PULP) are correctly updated to pass
defaultOperatorDescriptors, ensuring consistent integration of the new descriptor-based deployment model.Also applies to: 111-111, 127-127, 143-143
Deeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.py (1)
34-41: LGTM! Type casting ensures consistency with canonicalized attributes.The explicit
int()casts fortransAandtransBalign with the descriptor-driven canonicalization introduced in this PR, where attributes are normalized to consistent types (tuples). This defensive casting ensures the dimension index calculations work correctly regardless of the attribute's original representation.Deeploy/Targets/PULPOpen/Templates/FloatGemmTemplate.py (1)
23-24: LGTM! Integer casting in template aligns with canonicalization.The explicit
int()casts ensure the generated C code passes integer values toPULP_Gemm_fp*functions, consistent with the descriptor-driven type normalization introduced across the codebase.Deeploy/Targets/Neureka/Parsers.py (2)
21-21: LGTM! Tuple comparison aligns with canonicalized attribute format.The change from list
[1, 1]to tuple(1, 1)reflects the descriptor system's canonicalization, which normalizes attributes to tuples for consistency.
132-132: LGTM! Consistent with canonicalized attribute format.As with the dilations check, comparing against
(1, 1)tuple matches the canonical form produced by the descriptor system.Deeploy/Targets/SoftHier/Deployer.py (1)
11-11: LGTM! Standard descriptor propagation pattern.The addition of
operatorDescriptorsparameter and its forwarding to the base class follows the consistent pattern applied across all deployers in this PR.Also applies to: 21-21, 27-28
Deeploy/Targets/Chimera/Deployer.py (1)
11-11: LGTM! Consistent descriptor propagation.The changes follow the same pattern applied across all deployer classes, properly threading
operatorDescriptorsthrough the initialization chain.Also applies to: 21-21, 31-31
Deeploy/Targets/Neureka/Deployer.py (1)
12-12: LGTM! Standard descriptor parameter addition.The
operatorDescriptorsparameter is properly added and forwarded, matching the pattern applied uniformly across the codebase.Also applies to: 25-25, 31-32
Deeploy/Targets/Generic/Deployer.py (1)
14-14: LGTM! Descriptor propagation implemented correctly.The addition and forwarding of
operatorDescriptorsis consistent with the deployment framework's new descriptor-driven architecture.Also applies to: 25-25, 36-36
Deeploy/Targets/Snitch/Deployer.py (1)
13-13: LGTM! Operator descriptor parameter properly threaded through.The addition of
operatorDescriptorsto the constructor signature and its forwarding to the base class is consistent with the broader refactor introducing the OperatorDescriptor system across deployers.Also applies to: 25-25, 35-35
Deeploy/Targets/MemPool/Deployer.py (1)
14-14: LGTM! Operator descriptor parameter correctly propagated.The operatorDescriptors parameter is properly added and forwarded to the base class, consistent with the system-wide descriptor propagation pattern.
Also applies to: 25-25, 31-31
Deeploy/MemoryLevelExtension/NetworkDeployers/MemoryLevelDeployer.py (2)
14-15: LGTM! MemoryLevelAwareDeployer properly updated.The operatorDescriptors parameter is correctly added to MemoryLevelAwareDeployer and forwarded to the base class.
Also applies to: 115-115, 121-121
159-159: LGTM! MemoryLevelAwareSignPropDeployer properly updated.The operatorDescriptors parameter is correctly added to MemoryLevelAwareSignPropDeployer and forwarded to the base class.
Also applies to: 166-166
Deeploy/Targets/CortexM/Deployer.py (1)
14-14: LGTM! Operator descriptor parameter correctly threaded through.The operatorDescriptors parameter is properly added and forwarded to the base class.
Also applies to: 25-25, 36-36
Deeploy/Targets/PULPOpen/Deployer.py (1)
15-16: LGTM! Operator descriptor parameter correctly propagated.The operatorDescriptors parameter is properly threaded through to the base class.
Also applies to: 37-37, 47-47
Deeploy/CommonExtensions/NetworkDeployers/SignPropDeployer.py (1)
10-10: LGTM! SignPropDeployer base class properly updated.The operatorDescriptors parameter is correctly added to the base SignPropDeployer class and forwarded to NetworkDeployer, ensuring all derived deployers can access operator descriptor metadata.
Also applies to: 21-21, 27-27
Deeploy/EngineExtension/NetworkDeployers/EngineColoringDeployer.py (1)
11-12: LGTM! EngineColoringDeployer correctly updated.The operatorDescriptors parameter is properly threaded through to the base NetworkDeployer.
Also applies to: 24-24, 30-30
DeeployTest/testUtils/platformMapping.py (1)
10-10: LGTM! Clean propagation of operator descriptors.The changes systematically thread
operatorDescriptorsthrough all platform deployers with proper defaulting and type hints. The pattern is consistent across all 8 deployer branches.Also applies to: 13-13, 97-114, 128-128, 147-147, 166-166, 188-188, 207-207, 225-225, 242-242, 259-259
Deeploy/Targets/Generic/Bindings.py (1)
11-11: LGTM! ReduceMean bindings correctly updated for axes input.The addition of
PointerClass(int64_t)as the second input aligns with ONNX ReduceMean opset 18+, which requires axes as an input tensor rather than an attribute. Both signed integer and float paths are consistently updated.Also applies to: 197-203
Deeploy/Targets/Generic/Parsers.py (8)
55-56: LGTM! Direct attribute access simplifies parsing.Removing the unnecessary intermediate extraction and int() cast for
n_levelsis cleaner and more readable.
672-676: LGTM! Consistent direct attribute access pattern.The parsers now access
node.attrsdirectly, which is safe sinceparseNodevalidates attribute presence beforeparseNodeCtxtis called.Also applies to: 701-701, 728-729
851-853: LGTM! Consistent log2 computation pattern.Direct attribute access for
muland inline log2D computation follows the pattern established across other parsers.
1411-1411: LGTM! Bulk attribute propagation simplifies code.Using
update(node.attrs)to propagate all attributes at once is more maintainable than individual assignments.
1459-1478: LGTM! Systematic log2 conversion for requantization divisors.The pattern of bulk-copying attributes via
update(node.attrs)followed by post-processing to compute log2 equivalents for all*_divattributes is clear and maintainable.
1520-1520: LGTM! Consistent bulk attribute propagation.Using
update(node.attrs)follows the established pattern for complex operators with many attributes.
2376-2381: LGTM! Consistent bulk attribute update with log2 post-processing.The pattern of using
update(node.attrs)followed by computing log2 equivalents for*_divattributes is consistent with similar parsers (e.g., LinearAttentionParser).
2449-2452: LGTM! Direct attribute access for dequantization parameters.Removing unnecessary type conversions simplifies the code while maintaining correctness.
DeeployTest/testUtils/dmaUtils.py (1)
13-14: LGTM! Test infrastructure correctly updated for operator descriptors.The
memcpyDescaccurately describes the Memcpy operation with one required input (src) and one required output (dest), matching the validation logic inMemcpyParser. The descriptor is properly threaded through both PULP and Snitch test deployers.Also applies to: 282-290, 313-313, 355-355
Deeploy/DeeployTypes.py (2)
1041-1052: Variadic Io descriptor looks goodAPI and checks are straightforward; no issues.
1295-1297: Good extension of _unpack_constAllowing int/float/bool/str improves robustness for frontends emitting Python scalars.
Deeploy/OperatorDescriptor.py (2)
181-188: Axes canonicalization for ReduceMean: confirm expected formConverting attr→input for opset<18 will now leave both forms after the change above. Verify downstream parsers accept either; otherwise, gate this behind a feature flag or skip conversion here.
Would you prefer to only convert for opset>=18 graphs? If yes:
- if opset < 18: + if opset >= 18: if "axes" in node.attrs: attrToTensor(node, "axes")
89-99: Slice canonicalization looks fine given attr retentionWith attrs retained (see attrToTensor), this provides a unified input form without breaking parsers.
| self.operatorRepresentation['n_levels'] = node.attrs['n_levels'] | ||
| self.operatorRepresentation['signed'] = node.attrs['signed'] | ||
| self.operatorRepresentation['log2D'] = int(math.log2(node.attrs['div'])) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify attribute presence to avoid KeyError.
Line 189 checks for 'n_levels' in node.attrs OR 'n_levels_out' in node.attrs, but line 205 directly accesses node.attrs['n_levels'] without checking which variant exists. If only n_levels_out is present, this will raise a KeyError. Similarly, node.attrs['div'] is accessed without verification.
Consider verifying the attribute access pattern:
🏁 Script executed:
#!/bin/bash
# Search for RequantizedConv nodes and their attribute usage patterns
rg -nC5 "RequantizedConv|n_levels_out|n_levels" --type=py -g "!**/test/**"Length of output: 116503
I need to verify whether canonicalization happens before the PULPOpen parser runs. Let me search for the execution flow:
Let me search the codebase for when canonicalize is called and how parseNode is invoked:
Based on my analysis of the grep output, the review comment is correct about the issue.
Key Findings:
-
Lines 205-207 in PULPOpen/Parsers.py directly access
node.attrs['n_levels']without checking if it exists. -
Line 189 only validates presence: The validation
any(['n_levels' in node.attrs, 'n_levels_out' in node.attrs])confirms that at least one variant exists, but not which one. -
Contrasting with Generic Parsers: Looking at
Deeploy/Targets/Generic/Parsers.py, lines 84-90 show the correct pattern:'n_levels' in node.attrs or 'n_levels_out' in node.attrs, ... n_levels = node.attrs['n_levels'] if 'n_levels' in node.attrs else node.attrs['n_levels_out']
This is the defensive approach PULPOpen should adopt.
-
Canonicalization context: While
RequantizedOperatorDescriptor.canonicalize()normalizesn_levels_out→n_levels(visible in OperatorDescriptor.py), relying on this externally-managed precondition in PULPOpen'sparseNodeis fragile and violates defensive programming principles.
Fix Required: Lines 205-207 should check which attribute exists and use it accordingly, not assume n_levels is present.
🤖 Prompt for AI Agents
In Deeploy/Targets/PULPOpen/Parsers.py around lines 205-207, the code
unconditionally reads node.attrs['n_levels'] which can be missing; change it to
defensively choose n_levels = node.attrs['n_levels'] if present else
node.attrs['n_levels_out'], then assign self.operatorRepresentation['n_levels']
= n_levels; keep the existing assignments for 'signed' and 'log2D' but ensure
you reference the chosen n_levels variable rather than assuming the 'n_levels'
key exists.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Deeploy/Targets/Generic/Parsers.py (2)
139-147: Ensure Slice steps/axes constants are integer-typednp.ones defaults to float64; steps should be integer. Cast to int64 for ONNX compliance.
- if len(node.inputs) <= 3: - values = np.array(list(range(self.operatorRepresentation['dims']))) + if len(node.inputs) <= 3: + values = np.array(list(range(self.operatorRepresentation['dims'])), dtype=np.int64) @@ - if len(node.inputs) <= 4: - values = np.ones((self.operatorRepresentation['dims'])) + if len(node.inputs) <= 4: + values = np.ones((self.operatorRepresentation['dims']), dtype=np.int64)
1626-1632: Bug: uses loop variable inputNode outside scope when hoisting CWhen bias hoisting is enabled, dtype and type are taken from the last loop variable rather than a specific tensor. Use A (or output) as the reference.
- if not self.noBiasHoisting: - values = np.zeros(ctxt.lookup(node.inputs[0].name).shape, dtype = inputNode.dtype) + if not self.noBiasHoisting: + A_buf = ctxt.lookup(node.inputs[0].name) + assert isinstance(A_buf, VariableBuffer) + values = np.zeros(A_buf.shape, dtype=np.asarray(0, dtype=np.float32).dtype) # or infer from A_buf zeroTensor = gs.Constant(f'{node.name}_C_Tensor', values = values) - ctxt.hoistConstant(zeroTensor, _type = ctxt.lookup(inputNode.name)._type) + ctxt.hoistConstant(zeroTensor, _type=A_buf._type) node.inputs.append(zeroTensor) self.operatorRepresentation['C'] = f'{node.name}_C_Tensor'And consider inferring dtype from A_buf or from target type rather than numpy default.
Also applies to: 1633-1645
♻️ Duplicate comments (5)
Deeploy/DeeployTypes.py (2)
1023-1032: Fix mutable default and normalize inputs/optionals defensivelyoptional=[] is shared across instances and both lists aren’t copied. Normalize to lists and avoid shared state.
Apply:
-class IoDesc: - - def __init__(self, required: Union[str, List[str]], optional: Union[str, List[str]] = []) -> None: - if isinstance(required, str): - required = [required] - self.required = required - if isinstance(optional, str): - optional = [optional] - self.optional = optional +class IoDesc: + + def __init__(self, required: Union[str, List[str]], optional: Optional[Union[str, List[str]]] = None) -> None: + if isinstance(required, str): + required = [required] + self.required = list(required) + if optional is None: + optional = [] + self.optional = [optional] if isinstance(optional, str) else list(optional)
1150-1169: Import itertools, fix typo, and report actual overlapping keysitertools is used but not imported; “tesnors” typo; overlapping keys should report intersection, not symmetric difference.
Apply:
@@ -from functools import reduce +from functools import reduce +import itertools @@ - opReprs = { - "input tensors": self.parseTensors(ctxt, node.inputs, self.inputDescriptor), - "output tesnors": self.parseTensors(ctxt, node.outputs, self.outputDescriptor), - "attributes": self.parseAttrs(node), - } + opReprs = { + "input tensors": self.parseTensors(ctxt, node.inputs, self.inputDescriptor), + "output tensors": self.parseTensors(ctxt, node.outputs, self.outputDescriptor), + "attributes": self.parseAttrs(node), + } @@ - for (firstName, firstOpRepr), (secondName, secondOpRepr) in itertools.combinations(opReprs.items(), 2): - firstKeySet = set(firstOpRepr.keys()) - secondKeySet = set(secondOpRepr.keys()) - assert firstKeySet.isdisjoint(secondKeySet), \ - f"[PARSE ERROR] (Node: {node.name}, Op: {node.op}) " \ - f"Keys from parsing {firstName} clash with the keys from parsing {secondName}. "\ - f"Overlapping keys: {firstKeySet ^ secondKeySet}" + for (firstName, firstOpRepr), (secondName, secondOpRepr) in itertools.combinations(opReprs.items(), 2): + firstKeySet = set(firstOpRepr.keys()) + secondKeySet = set(secondOpRepr.keys()) + overlap = firstKeySet & secondKeySet + assert not overlap, ( + f"[PARSE ERROR] (Node: {node.name}, Op: {node.op}) " + f"Keys from parsing {firstName} clash with the keys from parsing {secondName}. " + f"Overlapping keys: {sorted(overlap)}" + )Deeploy/OperatorDescriptor.py (3)
54-58: Avoid bare except; narrow exception typesCatching all exceptions can mask bugs like KeyboardInterrupt or SystemExit.
Apply:
def IntTupleIfNotSingleItemUnpack(value: Any) -> Union[int, Tuple[int, ...]]: try: return IntUnpack(value) - except: + except (TypeError, ValueError): return IntTupleUnpack(value)Based on static analysis hints.
61-70: Do not pop attributes during canonicalization (breaks existing parsers)Popping
node.attrs[attr]after creating a tensor breaks parsers that still read canonicalized attributes like 'axes'. The attribute must remain until all parsers migrate to consuming the canonical input form.Apply:
def attrToTensor(node: gs.Node, attr: str) -> None: values = node.attrs[attr] if isinstance(values, (int, float)): values = np.array([values]) elif isinstance(values, (list, tuple)): values = np.array(values) assert isinstance(values, np.ndarray), f"Unsupported values type {type(values)}" tensor = gs.Constant(f"{node.name}_{attr}", values) node.inputs.append(tensor) - node.attrs.pop(attr) + # Keep attribute for backward compatibility with parsers expecting attrs. + # Remove once all parsers consume the canonical input form.
696-717: Duplicate attribute descriptor: rqs1_n_levels listed twiceLine 705 duplicates line 700. Based on the pattern for rqs1, rqs2, and rqsOut tensors, line 705 should be
"rqs2_n_levels".Apply:
AttrDesc("rqs1_n_levels", IntUnpack), AttrDesc("rqs1_mul", IntUnpack), AttrDesc("rqs1_add", IntUnpack), AttrDesc("rqs1_div", IntUnpack), AttrDesc("rqs1_signed", BoolUnpack), - AttrDesc("rqs1_n_levels", IntUnpack), + AttrDesc("rqs2_n_levels", IntUnpack), AttrDesc("rqs2_mul", IntUnpack),
🧹 Nitpick comments (8)
Deeploy/Targets/PULPOpen/Bindings.py (1)
155-158: Reshape shape tensor as int64_t: likely correct; verify int32 shapes upcastONNX Reshape uses int64 shapes; using Pointer(int64_t) is consistent. If some models provide int32 shape constants, ensure type checker upcasts or canonicalization casts constants to int64 to avoid binding misses.
Deeploy/DeeployTypes.py (1)
1113-1118: Membership test style and logging, not printUse “not in” and log instead of print for consistency.
- for attrDesc in self.attrDescriptors: - if attrDesc.default is None and not attrDesc.name in node.attrs: - # TODO: Change to logging - print(f"[ERROR OP {node.op}] Missing attribute {attrDesc.name}") - valid = False + for attrDesc in self.attrDescriptors: + if attrDesc.default is None and attrDesc.name not in node.attrs: + log.error(f"[ERROR OP {node.op}] Missing attribute {attrDesc.name}") + valid = FalseDeeploy/Targets/Generic/Parsers.py (5)
851-853: Guard log2 computation against non‑power‑of‑two Dint(math.log2(D)) silently floors. If D must be a power of two, assert it; otherwise compute via round.
- self.operatorRepresentation['log2D'] = int(math.log2(node.attrs['D'])) + D = int(node.attrs['D']) + assert D > 0 and (D & (D - 1)) == 0, f"D must be power-of-two, got {D}" + self.operatorRepresentation['log2D'] = int(math.log2(D))
1646-1660: MatMul dims/batch inference: solid; consider renaming O and exposing batchesLogic is correct. As a tiny readability nit, rename O → K or N_out to avoid ambiguous single-letter.
Also applies to: 1665-1682
1434-1454: *De‑duplicate _div list and robustly coerce to log2The list contains duplicates; also guard against non‑ints.
- log2Attrs = [ - "preattn_requant_div", - "preattn_requant_div", - "normalizer_requant_div", - "normalizer_requant_div", - "postattn_requant_div", - "postattn_requant_div", - "wo_requant_div", - "wq_requant_div", - "wk_requant_div", - "wv_requant_div", - ] - - for attr in log2Attrs: - value = self.operatorRepresentation[attr] - assert isinstance(value, int) - self.operatorRepresentation[attr] = int(math.log2(value)) + for attr in { + "preattn_requant_div","normalizer_requant_div","postattn_requant_div", + "wo_requant_div","wq_requant_div","wk_requant_div","wv_requant_div" + }: + value = int(self.operatorRepresentation[attr]) + assert value > 0 and (value & (value - 1)) == 0, f"{attr} must be power-of-two, got {value}" + self.operatorRepresentation[attr] = int(math.log2(value))
2351-2357: Validate RQ divisors are powers of two before log2*Avoid silent floor by enforcing pow‑2.
- for tensor in ["rqs1", "rqs2", "rqsOut"]: - value = self.operatorRepresentation[f"{tensor}_div"] - assert isinstance(value, int) - self.operatorRepresentation[f"{tensor}_log2D"] = int(math.log2(value)) + for tensor in ["rqs1", "rqs2", "rqsOut"]: + value = int(self.operatorRepresentation[f"{tensor}_div"]) + assert value > 0 and (value & (value - 1)) == 0, f"{tensor}_div must be power-of-two, got {value}" + self.operatorRepresentation[f"{tensor}_log2D"] = int(math.log2(value))
55-57: Same note as above: guard log2(D) for iRMSNormIf D isn’t a power of two, int(math.log2(D)) floors. Add a pow‑2 check.
Deeploy/OperatorDescriptor.py (1)
23-23: Consider extracting exception message to a constant (optional)The static analysis tool suggests keeping exception messages short or defining them at the class level. For this simple case, the inline message is acceptable, but if these validators are extended, consider a consistent error-reporting pattern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Deeploy/CommonExtensions/DataTypes.py(1 hunks)Deeploy/DeeployTypes.py(7 hunks)Deeploy/OperatorDescriptor.py(1 hunks)Deeploy/Targets/Generic/Parsers.py(13 hunks)Deeploy/Targets/PULPOpen/Bindings.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
Deeploy/Targets/Generic/Parsers.py (1)
Deeploy/DeeployTypes.py (4)
parseNodeCtxt(1200-1225)NetworkContext(508-1020)lookup(720-752)VariableBuffer(232-360)
Deeploy/DeeployTypes.py (2)
Deeploy/OperatorDescriptor.py (5)
canonicalize(91-98)canonicalize(183-187)canonicalize(377-387)canonicalize(482-490)canonicalize(678-693)Deeploy/MemoryLevelExtension/NetworkDeployers/MemoryLevelDeployer.py (1)
lookup(74-75)
Deeploy/OperatorDescriptor.py (1)
Deeploy/DeeployTypes.py (7)
AttrDesc(1055-1079)IoDesc(1023-1038)OperatorDescriptor(1083-1169)VariadicIoDesc(1041-1051)inputs(2656-2673)canonicalize(1121-1132)outputs(2675-2692)
Deeploy/Targets/PULPOpen/Bindings.py (3)
Deeploy/CommonExtensions/DataTypes.py (5)
float32_t(74-78)int8_t(12-15)int32_t(24-27)int64_t(30-33)uint8_t(36-39)Deeploy/Targets/Generic/TypeCheckers.py (1)
ReshapeChecker(122-136)Deeploy/AbstractDataTypes.py (1)
PointerClass(536-559)
🪛 Ruff (0.14.1)
Deeploy/Targets/Generic/Parsers.py
998-998: Unused method argument: channels_first
(ARG002)
1652-1652: Ambiguous variable name: O
(E741)
1654-1654: Ambiguous variable name: O
(E741)
Deeploy/DeeployTypes.py
1025-1025: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
1114-1114: Test for membership should be not in
Convert to not in
(E713)
1131-1131: Avoid specifying long messages outside the exception class
(TRY003)
1157-1157: Undefined name itertools
(F821)
2744-2744: Avoid specifying long messages outside the exception class
(TRY003)
Deeploy/OperatorDescriptor.py
23-23: Avoid specifying long messages outside the exception class
(TRY003)
57-57: Do not use bare except
(E722)
🔇 Additional comments (8)
Deeploy/CommonExtensions/DataTypes.py (1)
90-94: Good fix: build a flat, ordered tupletuple(sorted(...)) removes the accidental 1‑element tuple-of-list and preserves ordering by typeWidth. Looks correct.
Deeploy/Targets/PULPOpen/Bindings.py (1)
12-13: Imports align with unified reshape bindingsBringing in FloatDataTypes and int64_t matches the new binding comprehension.
Deeploy/DeeployTypes.py (3)
1295-1297: Nice: accept Python primitives in _unpack_constThis reduces friction when attributes are plain literals.
2583-2609: Descriptor-plumbing is solid; ensure all ops have descriptorsAsserting presence of a descriptor before mapping is good. Please confirm “Constant” and any backend-internal ops either have descriptors or are removed in preprocessing to avoid assertion trips.
2738-2747: Canonicalize then check: good orderingCalling canonicalize prior to check ensures attribute normalization (e.g., axes handling) before validation.
Deeploy/Targets/Generic/Parsers.py (1)
989-1006: Unsqueeze now requires axes attr: confirm canonicalization orderRequiring axes in node.attrs is fine if descriptor canonicalization (opset ≥13 axes input → attr) runs before these parsers. Please confirm parsers are only invoked post-canonicalization.
Deeploy/OperatorDescriptor.py (2)
737-790: Well-structured operator descriptor registryThe
defaultOperatorDescriptorsregistry provides comprehensive coverage of standard ONNX operators and custom quantized variants. The consistent naming and organization make the registry maintainable.
336-358: Excellent pattern for computing spatial dimension defaultsThe helper functions (
_dilationsDefault,_kernelShapeDefault,_stridesDefault,_padsDefault) compute defaults dynamically from input tensor shapes, properly handling different spatial dimensions (1D, 2D, 3D convolutions). This is more robust than hardcoded defaults.
…e map axes to int64_t as per onnx
…nto node attributes to ommit creating a buffer for it
lukamac
force-pushed
the
parser-refactor-operator-descriptors
branch
from
ab3eb3e to
cd62a69
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
Deeploy/Targets/PULPOpen/Parsers.py (3)
119-127: Defensive attr access + safe integer log2 (DWConv1D).Guard presence, support n_levels_out fallback, and compute log2D without float rounding.
Apply:
@@ - ret = all([ + ret = all([ @@ - len(node.inputs) == 4, + len(node.inputs) == 4, + # Required RQS attrs + any(['n_levels' in node.attrs, 'n_levels_out' in node.attrs]), + 'signed' in node.attrs, + 'div' in node.attrs, ]) @@ - self.operatorRepresentation['n_levels'] = node.attrs['n_levels'] - self.operatorRepresentation['signed'] = node.attrs['signed'] - self.operatorRepresentation['log2D'] = int(math.log2(node.attrs['div'])) + n_levels = node.attrs['n_levels'] if 'n_levels' in node.attrs else node.attrs['n_levels_out'] + self.operatorRepresentation['n_levels'] = int(n_levels) + self.operatorRepresentation['signed'] = bool(node.attrs['signed']) + div = int(node.attrs['div']) + if div <= 0 or (div & (div - 1)) != 0: + raise ValueError(f"{node.name}: 'div' must be a positive power-of-two; got {div}") + self.operatorRepresentation['log2D'] = div.bit_length() - 1Also applies to: 136-139
212-219: Forward channels_first to super().parseNodeCtxt.Current call drops the argument; channels_last flows may parse incorrectly.
Apply:
- newCtxt, ret = super().parseNodeCtxt(ctxt, node) + newCtxt, ret = super().parseNodeCtxt(ctxt, node, channels_first)
247-252: Fix channels_last input indexing.Else-branch uses channels_first indices for inputs; yields wrong dimensions.
Apply:
- else: - self.operatorRepresentation['ch_im_in'] = data_in.shape[1] - self.operatorRepresentation['dim_im_in_x'] = data_in.shape[2] - self.operatorRepresentation['dim_im_in_y'] = data_in.shape[3] + else: + # NHWC + self.operatorRepresentation['ch_im_in'] = data_in.shape[3] + self.operatorRepresentation['dim_im_in_x'] = data_in.shape[1] + self.operatorRepresentation['dim_im_in_y'] = data_in.shape[2] self.operatorRepresentation['ch_im_out'] = data_out.shape[3] self.operatorRepresentation['dim_im_out_x'] = data_out.shape[1] self.operatorRepresentation['dim_im_out_y'] = data_out.shape[2]Deeploy/Targets/Neureka/Deployer.py (1)
20-31: Avoid mutable default for inputOffsets; pass a copy downstream.Use Optional and normalize before super().init.
Apply:
-from typing import Callable, Dict, Type +from typing import Callable, Dict, Optional, Type @@ - inputOffsets = {}): - super().__init__(graph, deploymentPlatform, inputTypes, loweringOptimizer, operatorDescriptors, scheduler, name, - default_channels_first, deeployStateDir, inputOffsets) + inputOffsets: Optional[Dict[str, int]] = None): + _inputOffsets = {} if inputOffsets is None else dict(inputOffsets) + super().__init__(graph, deploymentPlatform, inputTypes, loweringOptimizer, operatorDescriptors, scheduler, name, + default_channels_first, deeployStateDir, _inputOffsets)As per static analysis hints (B006).
Deeploy/Targets/Generic/Deployer.py (1)
20-41: inputOffsets not forwarded to base; default is mutable.This drops caller offsets and can change sign handling.
Apply:
-from typing import Callable, Dict, Type +from typing import Callable, Dict, Optional, Type @@ - inputOffsets: Dict[str, int] = {}): + inputOffsets: Optional[Dict[str, int]] = None): @@ - default_channels_first = default_channels_first, - deeployStateDir = deeployStateDir) + default_channels_first = default_channels_first, + deeployStateDir = deeployStateDir, + inputOffsets = ({} if inputOffsets is None else dict(inputOffsets)))As per static analysis hints (B006).
Deeploy/Targets/PULPOpen/Deployer.py (1)
32-53: Avoid mutable default for inputOffsets; normalize before super.Prevents shared state across instances.
Apply:
-from typing import Callable, Dict, List, Type +from typing import Callable, Dict, List, Optional, Type @@ - inputOffsets = {}): + inputOffsets: Optional[Dict[str, int]] = None): @@ - deeployStateDir = deeployStateDir, - inputOffsets = inputOffsets) + deeployStateDir = deeployStateDir, + inputOffsets = ({} if inputOffsets is None else dict(inputOffsets)))As per static analysis hints (B006).
Deeploy/MemoryLevelExtension/NetworkDeployers/MemoryLevelDeployer.py (2)
108-126: Avoid mutable default lists; construct after super().Also accept Optional for operatorDescriptors and inputOffsets consistently.
Apply:
-from typing import Any, Callable, Dict, List, Tuple, Type, Union +from typing import Any, Callable, Dict, List, Optional, Tuple, Type, Union @@ - memoryLevelAnnotationPasses: List[NetworkOptimizationPass] = []): - super().__init__(graph, deploymentPlatform, inputTypes, loweringOptimizer, operatorDescriptors, scheduler, name, - default_channels_first, deeployStateDir) - if len(memoryLevelAnnotationPasses) == 0: - memoryLevelAnnotationPasses.append(AnnotateDefaultMemoryLevel(self.Platform.memoryHierarchy)) - self.memoryLevelAnnotationOptimizer = NetworkOptimizer(memoryLevelAnnotationPasses) + memoryLevelAnnotationPasses: Optional[List[NetworkOptimizationPass]] = None): + super().__init__(graph, deploymentPlatform, inputTypes, loweringOptimizer, operatorDescriptors, scheduler, name, + default_channels_first, deeployStateDir) + passes = (list(memoryLevelAnnotationPasses) + if memoryLevelAnnotationPasses is not None + else [AnnotateDefaultMemoryLevel(self.Platform.memoryHierarchy)]) + self.memoryLevelAnnotationOptimizer = NetworkOptimizer(passes)As per static analysis hints (B006).
152-168: Optional inputOffsets and immutable defaults (SignProp variant).Mirror the Optional pattern and safe list handling.
Apply:
- loweringOptimizer: TopologyOptimizer, - operatorDescriptors: Dict[str, OperatorDescriptor], + loweringOptimizer: TopologyOptimizer, + operatorDescriptors: Dict[str, OperatorDescriptor], scheduler: Callable = lambda x: x, name: str = 'DeeployNetwork', default_channels_first: bool = True, - deeployStateDir: str = "DeeployState", - inputOffsets: Dict[str, int] = {}, - memoryLevelAnnotationPasses: List[NetworkOptimizationPass] = []): - super().__init__(graph, deploymentPlatform, inputTypes, loweringOptimizer, operatorDescriptors, scheduler, name, - default_channels_first, deeployStateDir, inputOffsets) - if len(memoryLevelAnnotationPasses) == 0: - memoryLevelAnnotationPasses.append(AnnotateDefaultMemoryLevel(self.Platform.memoryHierarchy)) - self.memoryLevelAnnotationOptimizer = NetworkOptimizer(memoryLevelAnnotationPasses) + deeployStateDir: str = "DeeployState", + inputOffsets: Optional[Dict[str, int]] = None, + memoryLevelAnnotationPasses: Optional[List[NetworkOptimizationPass]] = None): + super().__init__(graph, deploymentPlatform, inputTypes, loweringOptimizer, operatorDescriptors, scheduler, name, + default_channels_first, deeployStateDir, ({} if inputOffsets is None else dict(inputOffsets))) + passes = (list(memoryLevelAnnotationPasses) + if memoryLevelAnnotationPasses is not None + else [AnnotateDefaultMemoryLevel(self.Platform.memoryHierarchy)]) + self.memoryLevelAnnotationOptimizer = NetworkOptimizer(passes)As per static analysis hints (B006).
Deeploy/CommonExtensions/NetworkDeployers/SignPropDeployer.py (1)
16-35: Avoid mutable default and mutate‑safe offsets initialization.Current code mutates the passed dict and leaves missing keys untouched.
Apply:
-from typing import Callable, Dict, Type +from typing import Callable, Dict, Optional, Type @@ - deeployStateDir: str = "DeeployState", - inputOffsets: Dict[str, int] = {}): - super().__init__(graph, deploymentPlatform, inputTypes, loweringOptimizer, operatorDescriptors, scheduler, name, - default_channels_first, deeployStateDir) - - if inputOffsets == {}: - for key in inputTypes.keys(): - inputOffsets[key] = 0 - - self.inputOffsets = inputOffsets + deeployStateDir: str = "DeeployState", + inputOffsets: Optional[Dict[str, int]] = None): + super().__init__(graph, deploymentPlatform, inputTypes, loweringOptimizer, operatorDescriptors, scheduler, name, + default_channels_first, deeployStateDir) + + base = {} if inputOffsets is None else dict(inputOffsets) # copy; do not mutate caller + # Ensure all inputs present with a default of 0 + self.inputOffsets = {k: base.get(k, 0) for k in inputTypes.keys()}As per static analysis hints (B006).
Deeploy/Targets/MemPool/Deployer.py (1)
20-35: Avoid mutable default and forward inputOffsets to base.Aligns with other deployers and prevents shared state.
Apply:
-from typing import Callable, Dict, Type +from typing import Callable, Dict, Optional, Type @@ - deeployStateDir: str = "DeeployState", - inputOffsets: Dict[str, int] = {}): - super().__init__(graph, deploymentPlatform, inputTypes, loweringOptimizer, operatorDescriptors, scheduler, name, - default_channels_first, deeployStateDir) - - self.inputOffsets = inputOffsets + deeployStateDir: str = "DeeployState", + inputOffsets: Optional[Dict[str, int]] = None): + super().__init__(graph, deploymentPlatform, inputTypes, loweringOptimizer, operatorDescriptors, scheduler, name, + default_channels_first, deeployStateDir, ({} if inputOffsets is None else dict(inputOffsets)))As per static analysis hints (B006).
Deeploy/Targets/Generic/Parsers.py (2)
1626-1632: MatMul: wrong bias tensor creation (dtype var leak, wrong shape, wrong type source).‘inputNode’ is the loop’s last var; here it’s undefined/unstable. Bias shape should match output, not A. Fix:
- if not self.noBiasHoisting: - values = np.zeros(ctxt.lookup(node.inputs[0].name).shape, dtype = inputNode.dtype) - zeroTensor = gs.Constant(f'{node.name}_C_Tensor', values = values) - ctxt.hoistConstant(zeroTensor, _type = ctxt.lookup(inputNode.name)._type) + if not self.noBiasHoisting: + out_shape = ctxt.lookup(node.outputs[0].name).shape + values = np.zeros(out_shape) + zeroTensor = gs.Constant(f'{node.name}_C_Tensor', values = values) + ctxt.hoistConstant(zeroTensor, _type = ctxt.lookup(node.outputs[0].name)._type) node.inputs.append(zeroTensor) self.operatorRepresentation['C'] = f'{node.name}_C_Tensor'
2219-2229: Unreachable code in GenericConv2DParser.parseNodeCtxt (dead assert + loop).Function already returned; remove tail.
if ret: return newCtxt, True else: return ctxt, False - - assert len(node.inputs - ) == 2, f'Supports only parsing 2 input tensors, data_in and weight. Received: {len(node.inputs)}' - for node, sym_name in zip(node.inputs, ['data_in', 'weight']): - self.operatorRepresentation[sym_name] = ctxt.lookup(node.name).name - - return newCtxt, True
♻️ Duplicate comments (9)
Deeploy/Targets/PULPOpen/Parsers.py (1)
176-191: Use n_levels_out fallback and require 'div' (DWConv2D).Directly indexing node.attrs['n_levels'] will KeyError when only n_levels_out exists; also 'div' presence is not validated. This was flagged before.
Apply:
@@ - len(node.inputs) == 4, - 'shift' in node.attrs, - any(['n_levels' in node.attrs, 'n_levels_out' in node.attrs]), - 'signed' in node.attrs + len(node.inputs) == 4, + 'shift' in node.attrs, + any(['n_levels' in node.attrs, 'n_levels_out' in node.attrs]), + 'signed' in node.attrs, + 'div' in node.attrs, ]) @@ - self.operatorRepresentation['n_levels'] = node.attrs['n_levels'] - self.operatorRepresentation['signed'] = node.attrs['signed'] - self.operatorRepresentation['log2D'] = int(math.log2(node.attrs['div'])) + n_levels = node.attrs['n_levels'] if 'n_levels' in node.attrs else node.attrs['n_levels_out'] + self.operatorRepresentation['n_levels'] = int(n_levels) + self.operatorRepresentation['signed'] = bool(node.attrs['signed']) + div = int(node.attrs['div']) + if div <= 0 or (div & (div - 1)) != 0: + raise ValueError(f"{node.name}: 'div' must be a positive power-of-two; got {div}") + self.operatorRepresentation['log2D'] = div.bit_length() - 1Also applies to: 205-207
Deeploy/Targets/Generic/Parsers.py (2)
661-677: Include ‘n_levels’ in iSoftmax required-attrs check (prevents KeyError).You read node.attrs['n_levels'] but don’t require it. Add it to the well‑formedness gate.
- if wellFormed: - wellFormed = all([ + if wellFormed: + wellFormed = all([ 'coeffA' in node.attrs, 'coeffB' in node.attrs, 'coeffC' in node.attrs, 'log2' in node.attrs, + 'n_levels' in node.attrs, ])
2423-2428: Dequant: tolerate missing ‘signed’; default to True (parity with Quant).Prevents KeyError and aligns behavior.
- self.operatorRepresentation['scale'] = node.attrs['scale'] - self.operatorRepresentation['zero_point'] = node.attrs['zero_point'] - self.operatorRepresentation['bit_width'] = node.attrs['bit_width'] - self.operatorRepresentation['signed'] = node.attrs['signed'] + self.operatorRepresentation['scale'] = node.attrs['scale'] + self.operatorRepresentation['zero_point'] = node.attrs['zero_point'] + self.operatorRepresentation['bit_width'] = node.attrs['bit_width'] + self.operatorRepresentation['signed'] = node.attrs.get('signed', True)Deeploy/OperatorDescriptor.py (4)
55-60: Narrow exception in IntTupleIfNotSingleItemUnpack.Avoid bare except; only catch expected conversion errors.
def IntTupleIfNotSingleItemUnpack(value: Any) -> Union[int, Tuple[int, ...]]: try: return IntUnpack(value) - except: + except (TypeError, ValueError): return IntTupleUnpack(value)
62-71: Don’t pop attrs in attrToTensor (backward‑compat with parsers reading attrs).Leave node.attrs[attr] intact after creating Constant.
tensor = gs.Constant(f"{node.name}_{attr}", values) node.inputs.append(tensor) - node.attrs.pop(attr) + # Keep attribute for backward compatibility; remove once all parsers consume canonical inputs.
501-520: Squeeze canonicalization: remove axes input after extracting to attrs.Descriptor expects single input; keep inputs consistent post‑canonicalization.
if len(node.inputs) == 2: axes = node.inputs[1] assert isinstance(axes, gs.Constant), \ f"Expected axes to be a constant but received axes of type {type(axes)}" node.attrs["axes"] = axes.values axes.outputs.clear() + node.inputs.pop(1) # drop axes tensor after extraction
728-745: Duplicate ‘rqs1_n_levels’ in RequantizedAdd attr list.Remove the duplicate entry.
AttrDesc("rqs1_div", IntUnpack), AttrDesc("rqs1_signed", BoolUnpack), - AttrDesc("rqs1_n_levels", IntUnpack), AttrDesc("rqs2_mul", IntUnpack),Deeploy/DeeployTypes.py (2)
1023-1032: IoDesc: avoid mutable default argument for ‘optional’.Use None and normalize to a new list.
-class IoDesc: - - def __init__(self, required: Union[str, List[str]], optional: Union[str, List[str]] = []) -> None: +class IoDesc: + + def __init__(self, required: Union[str, List[str]], + optional: Optional[Union[str, List[str]]] = None) -> None: if isinstance(required, str): - required = [required] - self.required = required - if isinstance(optional, str): - optional = [optional] - self.optional = optional + required = [required] + self.required = list(required) + if optional is None: + optional = [] + self.optional = [optional] if isinstance(optional, str) else list(optional)
1147-1154: OperatorDescriptor.parse: missing import and typo in key.Import itertools; fix “output tesnors” → “output tensors”.
-from functools import reduce +from functools import reduce +import itertools @@ - opReprs = { + opReprs = { "input tensors": self.parseTensors(ctxt, node.inputs, self.inputDescriptor), - "output tesnors": self.parseTensors(ctxt, node.outputs, self.outputDescriptor), + "output tensors": self.parseTensors(ctxt, node.outputs, self.outputDescriptor), "attributes": self.parseAttrs(node), }
🧹 Nitpick comments (7)
Deeploy/Targets/Generic/TypeCheckers.py (1)
188-189: Use a more descriptive variable name thanO.The logic correctly handles the
transAflag, but the variable nameOis ambiguous (capital letter O resembles zero 0). Consider using a more descriptive name for clarity.Apply this diff:
- O = inputs[0].shape[-1] if not operatorRepresentation['transA'] else inputs[0].shape[-2] - return [2**((self.input_types[0].referencedType.typeWidth) * 2) * O] + kernel_size = inputs[0].shape[-1] if not operatorRepresentation['transA'] else inputs[0].shape[-2] + return [2**((self.input_types[0].referencedType.typeWidth) * 2) * kernel_size]Deeploy/Targets/Generic/TopologyOptimizationPasses/Passes.py (1)
357-357: Consider usingnext(iter(...))for efficiency.Static analysis suggests using
next(iter(match.nodes_map.values()))instead oflist(match.nodes_map.values())[0]to avoid creating an intermediate list.Apply this diff:
- conv = list(match.nodes_map.values())[0] + conv = next(iter(match.nodes_map.values()))Deeploy/Targets/PULPOpen/Bindings.py (2)
156-158: Reshape: consider accepting int32 shape as well.ONNX uses int64, but some graphs provide int32 shapes; adding a fallback binding improves robustness.
Apply:
PULPReshapeBindings = [ NodeBinding(ReshapeChecker([PointerClass(type), PointerClass(int64_t)], [PointerClass(type)]), ReshapeTemplate.referenceTemplate, SkipTransformer) for type in IntegerDataTypes + FloatDataTypes -] +] + [ + NodeBinding(ReshapeChecker([PointerClass(type), PointerClass(int32_t)], [PointerClass(type)]), + ReshapeTemplate.referenceTemplate, SkipTransformer) for type in IntegerDataTypes + FloatDataTypes +]
62-75: Deduplicate Closure aliases (minor hygiene).FunctionCallClosure and ForkClosure are defined twice.
Apply:
-FunctionCallClosure = partial(ClosureGeneration, closureSuffix = "_closure") -ClusterClosure = partial(ClosureGeneration, +ClusterClosure = partial(ClosureGeneration, closureSuffix = "_cluster_entry", closureCallTemplate = _clusterEntryClosureCallTemplate) -ForkClosure = partial(ClosureGeneration, - closureSuffix = "_cluster_fork", - closureCallTemplate = _clusterForkClosureCallTemplate) +ForkClosure = partial(ClosureGeneration, + closureSuffix = "_cluster_fork", + closureCallTemplate = _clusterForkClosureCallTemplate) @@ -FunctionCallClosure = partial(ClosureGeneration, closureSuffix = "_closure") -ForkClosure = partial(ClosureGeneration, - closureSuffix = "_cluster_fork", - closureCallTemplate = _clusterForkClosureCallTemplate) +# (second duplicate removed)Deeploy/Targets/Generic/Parsers.py (2)
1651-1660: Rename ambiguous variable ‘O’ (ruff E741) and keep key stable.Improves readability without changing semantics.
- if self.operatorRepresentation['transB']: - O, N_B = buffB.shape[-2:] - else: - N_B, O = buffB.shape[-2:] + if self.operatorRepresentation['transB']: + out_dim, N_B = buffB.shape[-2:] + else: + N_B, out_dim = buffB.shape[-2:] @@ - self.operatorRepresentation['O'] = O + self.operatorRepresentation['O'] = out_dim
995-1007: Silence ARG002: explicitly mark ‘channels_first’ unused.Keeps signature while satisfying linter.
def parseNodeCtxt(self, ctxt: NetworkContext, node: gs.Node, channels_first: bool = True) -> Tuple[NetworkContext, bool]: - inputs = ['data_in'] + _ = channels_first + inputs = ['data_in']Deeploy/DeeployTypes.py (1)
1111-1114: Style: prefer ‘not in’ over ‘not X in’.No behavior change; silences E713.
- if attrDesc.default is None and not attrDesc.name in node.attrs: + if attrDesc.default is None and attrDesc.name not in node.attrs:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
CHANGELOG.md(2 hunks)Deeploy/CommonExtensions/DataTypes.py(1 hunks)Deeploy/CommonExtensions/NetworkDeployers/SignPropDeployer.py(2 hunks)Deeploy/DeeployTypes.py(7 hunks)Deeploy/EngineExtension/NetworkDeployers/EngineColoringDeployer.py(2 hunks)Deeploy/MemoryLevelExtension/NetworkDeployers/MemoryLevelDeployer.py(3 hunks)Deeploy/OperatorDescriptor.py(1 hunks)Deeploy/Targets/Chimera/Deployer.py(3 hunks)Deeploy/Targets/CortexM/Deployer.py(3 hunks)Deeploy/Targets/Generic/Bindings.py(2 hunks)Deeploy/Targets/Generic/Deployer.py(3 hunks)Deeploy/Targets/Generic/Parsers.py(13 hunks)Deeploy/Targets/Generic/Templates/FloatGemmTemplate.py(1 hunks)Deeploy/Targets/Generic/Templates/GemmTemplate.py(1 hunks)Deeploy/Targets/Generic/TopologyOptimizationPasses/Passes.py(1 hunks)Deeploy/Targets/Generic/TypeCheckers.py(1 hunks)Deeploy/Targets/MemPool/Deployer.py(2 hunks)Deeploy/Targets/MemPool/Templates/GemmTemplate.py(1 hunks)Deeploy/Targets/MemPool/Templates/RQGemmTemplate.py(2 hunks)Deeploy/Targets/MemPool/TopologyOptimizationPasses/Passes.py(1 hunks)Deeploy/Targets/Neureka/Deployer.py(2 hunks)Deeploy/Targets/Neureka/Parsers.py(2 hunks)Deeploy/Targets/PULPOpen/Bindings.py(2 hunks)Deeploy/Targets/PULPOpen/Deployer.py(3 hunks)Deeploy/Targets/PULPOpen/Parsers.py(2 hunks)Deeploy/Targets/PULPOpen/Templates/FloatGemmTemplate.py(1 hunks)Deeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.py(1 hunks)Deeploy/Targets/Snitch/Deployer.py(3 hunks)Deeploy/Targets/Snitch/Parsers.py(2 hunks)Deeploy/Targets/SoftHier/Deployer.py(2 hunks)DeeployTest/testMemoryLevelExtension.py(5 hunks)DeeployTest/testUtils/dmaUtils.py(4 hunks)DeeployTest/testUtils/platformMapping.py(11 hunks)
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (12)
- Deeploy/Targets/Chimera/Deployer.py
- Deeploy/Targets/Snitch/Deployer.py
- Deeploy/Targets/Snitch/Parsers.py
- Deeploy/CommonExtensions/DataTypes.py
- Deeploy/Targets/Generic/Templates/GemmTemplate.py
- DeeployTest/testUtils/platformMapping.py
- Deeploy/Targets/Generic/Bindings.py
- Deeploy/EngineExtension/NetworkDeployers/EngineColoringDeployer.py
- Deeploy/Targets/Generic/Templates/FloatGemmTemplate.py
- Deeploy/Targets/PULPOpen/Templates/FloatGemmTemplate.py
- DeeployTest/testMemoryLevelExtension.py
- DeeployTest/testUtils/dmaUtils.py
🧰 Additional context used
🧬 Code graph analysis (14)
Deeploy/Targets/Generic/Deployer.py (1)
Deeploy/DeeployTypes.py (3)
DeploymentPlatform(2525-2568)OperatorDescriptor(1083-1166)TopologyOptimizer(2323-2352)
Deeploy/Targets/CortexM/Deployer.py (1)
Deeploy/DeeployTypes.py (3)
DeploymentPlatform(2525-2568)OperatorDescriptor(1083-1166)TopologyOptimizer(2323-2352)
Deeploy/Targets/MemPool/Deployer.py (1)
Deeploy/DeeployTypes.py (3)
DeploymentPlatform(2525-2568)OperatorDescriptor(1083-1166)TopologyOptimizer(2323-2352)
Deeploy/Targets/PULPOpen/Deployer.py (1)
Deeploy/DeeployTypes.py (2)
OperatorDescriptor(1083-1166)VariableBuffer(232-360)
Deeploy/Targets/Generic/Parsers.py (1)
Deeploy/DeeployTypes.py (4)
parseNodeCtxt(1197-1222)NetworkContext(508-1020)lookup(720-752)VariableBuffer(232-360)
Deeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.py (1)
Deeploy/TilingExtension/TilerModel.py (1)
getTensorDimVar(131-135)
Deeploy/MemoryLevelExtension/NetworkDeployers/MemoryLevelDeployer.py (1)
Deeploy/DeeployTypes.py (5)
NetworkContext(508-1020)NetworkDeployer(3335-3715)OperatorDescriptor(1083-1166)TopologyOptimizer(2323-2352)VariableBuffer(232-360)
Deeploy/Targets/PULPOpen/Bindings.py (3)
Deeploy/CommonExtensions/DataTypes.py (5)
float32_t(74-78)int8_t(12-15)int32_t(24-27)int64_t(30-33)uint8_t(36-39)Deeploy/Targets/Generic/TypeCheckers.py (1)
ReshapeChecker(122-136)Deeploy/AbstractDataTypes.py (1)
PointerClass(536-559)
Deeploy/Targets/Generic/TypeCheckers.py (1)
Deeploy/AbstractDataTypes.py (1)
typeWidth(399-400)
Deeploy/OperatorDescriptor.py (1)
Deeploy/DeeployTypes.py (7)
AttrDesc(1055-1079)IoDesc(1023-1038)OperatorDescriptor(1083-1166)VariadicIoDesc(1041-1051)inputs(2653-2670)canonicalize(1118-1129)outputs(2672-2689)
Deeploy/CommonExtensions/NetworkDeployers/SignPropDeployer.py (1)
Deeploy/DeeployTypes.py (4)
DeploymentPlatform(2525-2568)NetworkDeployer(3335-3715)OperatorDescriptor(1083-1166)TopologyOptimizer(2323-2352)
Deeploy/DeeployTypes.py (1)
Deeploy/OperatorDescriptor.py (5)
canonicalize(92-99)canonicalize(184-188)canonicalize(397-406)canonicalize(501-520)canonicalize(708-721)
Deeploy/Targets/Neureka/Deployer.py (1)
Deeploy/DeeployTypes.py (3)
DeploymentPlatform(2525-2568)OperatorDescriptor(1083-1166)TopologyOptimizer(2323-2352)
Deeploy/Targets/SoftHier/Deployer.py (1)
Deeploy/DeeployTypes.py (3)
DeploymentPlatform(2525-2568)OperatorDescriptor(1083-1166)TopologyOptimizer(2323-2352)
🪛 Ruff (0.14.1)
Deeploy/Targets/MemPool/Deployer.py
30-30: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
Deeploy/Targets/Generic/Parsers.py
998-998: Unused method argument: channels_first
(ARG002)
1652-1652: Ambiguous variable name: O
(E741)
1654-1654: Ambiguous variable name: O
(E741)
Deeploy/MemoryLevelExtension/NetworkDeployers/MemoryLevelDeployer.py
120-120: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
164-164: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
165-165: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
Deeploy/Targets/Generic/TypeCheckers.py
188-188: Ambiguous variable name: O
(E741)
Deeploy/OperatorDescriptor.py
24-24: Avoid specifying long messages outside the exception class
(TRY003)
58-58: Do not use bare except
(E722)
Deeploy/CommonExtensions/NetworkDeployers/SignPropDeployer.py
26-26: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
Deeploy/Targets/Generic/TopologyOptimizationPasses/Passes.py
357-357: Prefer next(iter(match.nodes_map.values())) over single element slice
Replace with next(iter(match.nodes_map.values()))
(RUF015)
378-378: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
Deeploy/DeeployTypes.py
1025-1025: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
1112-1112: Test for membership should be not in
Convert to not in
(E713)
1128-1128: Avoid specifying long messages outside the exception class
(TRY003)
1154-1154: Undefined name itertools
(F821)
2741-2741: Avoid specifying long messages outside the exception class
(TRY003)
Deeploy/Targets/Neureka/Deployer.py
30-30: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
Deeploy/Targets/SoftHier/Deployer.py
26-26: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
⏰ 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). (100)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (12000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 60000, 10000, 5000, 8, L3) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (simpleRegression, 45000, 30000, 16000, 8, L3) / test-runner-siracusa-tiled (45000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-models / test-runner-siracusa
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: generic-models / test-runner-generic
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (12000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 60000, 10000, 5000, 8, L3) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (simpleRegression, 45000, 30000, 16000, 8, L3) / test-runner-siracusa-tiled (45000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-models / test-runner-siracusa
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: generic-models / test-runner-generic
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (12000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 60000, 10000, 5000, 8, L3) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (simpleRegression, 45000, 30000, 16000, 8, L3) / test-runner-siracusa-tiled (45000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-models / test-runner-siracusa
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: generic-models / test-runner-generic
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (12000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 60000, 10000, 5000, 8, L3) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (simpleRegression, 45000, 30000, 16000, 8, L3) / test-runner-siracusa-tiled (45000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-models / test-runner-siracusa
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: generic-models / test-runner-generic
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (12000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 60000, 10000, 5000, 8, L3) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (simpleRegression, 45000, 30000, 16000, 8, L3) / test-runner-siracusa-tiled (45000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-models / test-runner-siracusa
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: generic-models / test-runner-generic
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (12000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 60000, 10000, 5000, 8, L3) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (simpleRegression, 45000, 30000, 16000, 8, L3) / test-runner-siracusa-tiled (45000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-models / test-runner-siracusa
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: generic-models / test-runner-generic
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (12000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 60000, 10000, 5000, 8, L3) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (simpleRegression, 45000, 30000, 16000, 8, L3) / test-runner-siracusa-tiled (45000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-models / test-runner-siracusa
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: generic-models / test-runner-generic
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (12000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 60000, 10000, 5000, 8, L3) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (simpleRegression, 45000, 30000, 16000, 8, L3) / test-runner-siracusa-tiled (45000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-models / test-runner-siracusa
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: generic-models / test-runner-generic
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (12000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 60000, 10000, 5000, 8, L3) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (simpleRegression, 45000, 30000, 16000, 8, L3) / test-runner-siracusa-tiled (45000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-models / test-runner-siracusa
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: generic-models / test-runner-generic
- GitHub Check: chimera-kernels / test-runner-chimera
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (12000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (microLlama/microLlama1, 60000, 10000, 5000, 8, L3) / test-runner-siracusa-tiled (10000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (miniMobileNet, 60000, 12000, 6000, 8, L3) / test-runner-siracusa-tiled (60000)
- GitHub Check: siracusa-models-tiled-singlebuffer-L3 (simpleRegression, 45000, 30000, 16000, 8, L3) / test-runner-siracusa-tiled (45000)
- GitHub Check: siracusa-kernels / test-runner-siracusa
- GitHub Check: siracusa-models / test-runner-siracusa
- GitHub Check: siracusa-neureka-kernels-tiled-singlebuffer-L2 / test-runner-siracusa-neureka-tiled
- GitHub Check: generic-kernels / test-runner-generic
- GitHub Check: generic-models / test-runner-generic
- GitHub Check: chimera-kernels / test-runner-chimera
🔇 Additional comments (10)
Deeploy/Targets/MemPool/Templates/RQGemmTemplate.py (1)
148-149: LGTM! Consistent type handling for transposition flags.The explicit
int()casting oftransAandtransBensures proper type handling for C code generation and aligns with similar changes across other Gemm template variants in this PR.Also applies to: 173-174
Deeploy/Targets/CortexM/Deployer.py (1)
14-14: LGTM! Properly integrates OperatorDescriptor system.The changes correctly add and propagate the
operatorDescriptorsparameter through the CortexMDeployer, consistent with the descriptor-driven architecture introduced across other deployers in this PR.Also applies to: 25-25, 36-36
Deeploy/Targets/MemPool/TopologyOptimizationPasses/Passes.py (1)
292-292: LGTM! Keepdims value is correct.The
keepdims: 0setting is appropriate for dimension reduction in the ReduceSum operation.Deeploy/Targets/MemPool/Templates/GemmTemplate.py (1)
130-131: LGTM! Consistent type handling for transposition flags.The explicit
int()casting oftransAandtransBensures proper type handling for the C Gemm kernel call, matching the pattern applied across other Gemm templates in this PR.Deeploy/Targets/Generic/TopologyOptimizationPasses/Passes.py (3)
356-357: Return type annotation added.Good addition for type clarity.
359-365: LGTM! Early exits improve performance.The early returns for missing
padsattribute and zero padding are sensible optimizations.
367-396: Refactored padding extraction logic is clear.The consolidated approach with explicit variable naming (
convPads,inTensor,convShape) improves readability. The logic correctly constructs non-spatial padding and creates the Pad node.Note: At line 378, consider adding
strict=Truetozip()to catch potential length mismatches:- for dim, begin, end in zip(convShape, pads[:len(pads) // 2], pads[len(pads) // 2:]): + for dim, begin, end in zip(convShape, pads[:len(pads) // 2], pads[len(pads) // 2:], strict=True):Deeploy/Targets/SoftHier/Deployer.py (1)
11-11: LGTM! Properly integrates OperatorDescriptor system.The changes correctly add and propagate the
operatorDescriptorsparameter, consistent with the descriptor-driven architecture across other deployers.Also applies to: 21-21, 27-28
Deeploy/DeeployTypes.py (1)
2735-2743: Re-raise with message is fine; no action, just confirm.generate error path is already wrapped to dump state and rethrow. No change needed.
Deeploy/OperatorDescriptor.py (1)
775-830: Registry looks consistent; mapping “Pad” to padDescOld ensures opset ≤24 fallback.LGTM.
Deeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.py
Outdated
Show resolved
Hide resolved
lukamac
force-pushed
the
parser-refactor-operator-descriptors
branch
from
886aa28 to
97da07c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Deeploy/Targets/Generic/Parsers.py (2)
144-147: Use integer dtype for Slice steps tensor.np.ones defaults to float; ONNX Slice steps are integers.
- values = np.ones((self.operatorRepresentation['dims'])) + values = np.ones((self.operatorRepresentation['dims']), dtype = np.int64)
1625-1629: Fix dtype/source for hoisted C tensor in MatMulParser.Using loop var inputNode here is fragile; take dtype and type from A explicitly.
- values = np.zeros(ctxt.lookup(node.inputs[0].name).shape, dtype = inputNode.dtype) + values = np.zeros(ctxt.lookup(node.inputs[0].name).shape, dtype = node.inputs[0].dtype) zeroTensor = gs.Constant(f'{node.name}_C_Tensor', values = values) - ctxt.hoistConstant(zeroTensor, _type = ctxt.lookup(inputNode.name)._type) + ctxt.hoistConstant(zeroTensor, _type = ctxt.lookup(node.inputs[0].name)._type)
♻️ Duplicate comments (4)
Deeploy/OperatorDescriptor.py (2)
62-71: Don’t pop attributes when tensorizing them (avoid breaking parsers still reading attrs).Keep node.attrs[attr] after appending the Constant input to preserve backward compatibility. This was raised earlier; applying again for this function.
def attrToInputTensor(node: gs.Node, attr: str) -> None: @@ tensor = gs.Constant(f"{node.name}_{attr}", values) node.inputs.append(tensor) - node.attrs.pop(attr) + # Keep attribute for backward compatibility; remove once all parsers consume inputs.To confirm no parsers rely on this attr, you can scan for direct attr reads:
#!/bin/bash rg -nP -C2 "node\.attrs\[['\"][a-zA-Z_]+\['\"]\]" Deeploy/Targets/Generic/Parsers.py
509-525: Remove axes input after extraction in Squeeze canonicalization.You convert input[1] -> attr "axes" but leave an extra input; IoDesc("data_in") then fails OperatorDescriptor.check().
class SqueezeDescriptor(OperatorDescriptor): @@ def canonicalize(self, node: gs.Node, opset: int) -> bool: if len(node.inputs) == 2: inputTensorToAttr(node, tensorIdx = 1, attr = "axes") + # Drop the axes tensor from inputs after extracting to attrs + node.inputs.pop(1) @@ return super().canonicalize(node, opset)Deeploy/Targets/Generic/Parsers.py (2)
664-677: Include n_levels in iSoftmax well‑formedness gate to avoid KeyError.The code reads node.attrs['n_levels'] but the gate doesn’t require it.
- if wellFormed: - wellFormed = all([ + if wellFormed: + wellFormed = all([ 'coeffA' in node.attrs, 'coeffB' in node.attrs, 'coeffC' in node.attrs, 'log2' in node.attrs, + 'n_levels' in node.attrs, ]) @@ - self.operatorRepresentation['log2'] = node.attrs['log2'] - self.operatorRepresentation['n_levels'] = node.attrs['n_levels'] + self.operatorRepresentation['log2'] = node.attrs['log2'] + self.operatorRepresentation['n_levels'] = node.attrs['n_levels']
2422-2426: Handle optional ‘signed’ in Dequant (default to True).Avoids KeyError and matches QuantParser behavior.
- self.operatorRepresentation['scale'] = node.attrs['scale'] - self.operatorRepresentation['zero_point'] = node.attrs['zero_point'] - self.operatorRepresentation['bit_width'] = node.attrs['bit_width'] - self.operatorRepresentation['signed'] = node.attrs['signed'] + self.operatorRepresentation['scale'] = node.attrs['scale'] + self.operatorRepresentation['zero_point'] = node.attrs['zero_point'] + self.operatorRepresentation['bit_width'] = node.attrs['bit_width'] + self.operatorRepresentation['signed'] = node.attrs.get('signed', True)
🧹 Nitpick comments (5)
Deeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.py (1)
30-48: LGTM! Transpose-aware dimension indexing is correct.The conditional logic correctly handles transpose flags to compute the matrix dimension indices. The approach is clearer than inline arithmetic.
Optional: Extract helper to reduce duplication.
The dimension index computation is duplicated for A (lines 30-36) and B (lines 38-44). Consider extracting a helper function:
def _getMatrixDimIndices(rank: int, transposed: bool) -> Tuple[int, int]: """Compute first and second matrix dimension indices based on rank and transpose flag.""" if not transposed: return rank - 2, rank - 1 else: return rank - 1, rank - 2Then use:
firstDimIdxA, secondDimIdxA = _getMatrixDimIndices(rankA, parseDict['transA']) # ... firstDimIdxB, secondDimIdxB = _getMatrixDimIndices(rankB, parseDict['transB'])Deeploy/OperatorDescriptor.py (1)
55-59: Broaden fallback exception for robust unpacking.Catching only ValueError can miss TypeError when value isn’t iterable or is a 0‑dim scalar; include both.
def IntTupleIfNotSingleItemUnpack(value: Any) -> Union[int, Tuple[int, ...]]: - try: - return IntUnpack(value) - except ValueError: - return IntTupleUnpack(value) + try: + return IntUnpack(value) + except (TypeError, ValueError): + return IntTupleUnpack(value)Deeploy/Targets/Generic/Parsers.py (2)
90-93: Validate div > 0 before log2 in RQSParserInterface.Prevents math domain errors and clarifies expectations.
- self.operatorRepresentation['log2D'] = int(math.log2(NodeParser._unpack_const(node.attrs['div']))) + div = int(NodeParser._unpack_const(node.attrs['div'])) + assert div > 0, "'div' must be a positive integer" + self.operatorRepresentation['log2D'] = int(math.log2(div))
2352-2355: *Validate requant _div > 0 before log2 in RQAddParser.Aligns with LinearAttention fix; avoids domain errors.
- value = self.operatorRepresentation[f"{tensor}_div"] - assert isinstance(value, int) - self.operatorRepresentation[f"{tensor}_log2D"] = int(math.log2(value)) + value = self.operatorRepresentation[f"{tensor}_div"] + assert isinstance(value, int) and value > 0, f"{tensor}_div must be a positive integer" + self.operatorRepresentation[f"{tensor}_log2D"] = int(math.log2(value))Deeploy/DeeployTypes.py (1)
1113-1116: Usenot inidiom for membership test.Minor style fix; clearer and satisfies linters.
- for attrDesc in self.attrDescriptors: - if attrDesc.default is None and not attrDesc.name in node.attrs: + for attrDesc in self.attrDescriptors: + if attrDesc.default is None and attrDesc.name not in node.attrs: log.error(f"[OP {node.op}] Missing attribute {attrDesc.name}") valid = False
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Deeploy/DeeployTypes.py(8 hunks)Deeploy/OperatorDescriptor.py(1 hunks)Deeploy/Targets/Generic/Parsers.py(13 hunks)Deeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
Deeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.py (2)
Deeploy/DeeployTypes.py (1)
lookup(721-753)Deeploy/TilingExtension/TilerModel.py (2)
addTensorDimToModel(143-157)getTensorDimVar(131-135)
Deeploy/Targets/Generic/Parsers.py (1)
Deeploy/DeeployTypes.py (4)
parseNodeCtxt(1199-1224)NetworkContext(509-1021)lookup(721-753)VariableBuffer(233-361)
Deeploy/OperatorDescriptor.py (1)
Deeploy/DeeployTypes.py (7)
AttrDesc(1057-1081)IoDesc(1024-1040)OperatorDescriptor(1085-1168)VariadicIoDesc(1043-1053)inputs(2655-2672)outputs(2674-2691)canonicalize(1120-1131)
Deeploy/DeeployTypes.py (1)
Deeploy/OperatorDescriptor.py (5)
canonicalize(100-107)canonicalize(192-196)canonicalize(405-414)canonicalize(509-524)canonicalize(712-725)
🪛 Ruff (0.14.1)
Deeploy/Targets/Generic/Parsers.py
998-998: Unused method argument: channels_first
(ARG002)
1650-1650: Ambiguous variable name: O
(E741)
1652-1652: Ambiguous variable name: O
(E741)
Deeploy/OperatorDescriptor.py
24-24: Avoid specifying long messages outside the exception class
(TRY003)
Deeploy/DeeployTypes.py
1114-1114: Test for membership should be not in
Convert to not in
(E713)
1130-1130: Avoid specifying long messages outside the exception class
(TRY003)
2743-2743: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (3)
Deeploy/Targets/PULPOpen/TileConstraints/MatMulTileConstraint.py (3)
24-28: LGTM! Clean buffer setup.The refactored loop over buffers eliminates repetitive calls and improves maintainability.
50-62: LGTM! Batch dimension broadcasting is correctly implemented.The right-aligned batch dimension mapping (using
(rankOut - rankA) + dimIdx) correctly implements numpy-style broadcasting semantics for MatMul with batch dimensions.
76-88: LGTM! Policy constraint logic is consistent and correct.The rank-based dimension indexing with transpose handling is correctly applied in
addPolicyConstraint, consistent with the approach inaddGeometricalConstraint.Note: The previous review comment about missing
int()casts is no longer applicable. The refactoring to use conditional branches (if not parseDict['transA']:) instead of inline arithmetic eliminates the need for explicit type casts, as the transpose flags are now only used in boolean contexts.
2 participants
First PR on my quest to refactor Deeploy's parsing
This PR adds a new class called
OperatorDescriptorto Deeploy with the task of coalescing all the target-agnostic operator information.It describes the operators required and optional io tensors, and attributes.
It provides methods for:
In this PR we are using just the canonicalization and node checking, while parsing is left for a future PR.
Added
Fixed
PR Merge Checklist
develcommit and pointing todevel.CHANGELOG.mdfile has been updated.