Skip to content

Conversation

@maltanar
Copy link
Collaborator

@maltanar maltanar commented Nov 18, 2025

Incorporates/supersedes #216 and #206 - see main description for those two PRs.
In addition, addresses these review comments:

  • Please address the in-line comments about the versioning for channels-last ops. The issue is that the standard ONNX node itself is used under the hood for executing the channels-last ops, which means the opset version for channels-last needs to track the opset version for the standard ops. As a solution, the channels-last ops are now registered with several different opset versions (v1 for Conv, v1 and v10 for MaxPool, v1, v9 and v14 for BatchNormalization) to cater for newly introduced attributes in the standard. These attributes don't have to be declared in the channels-last op versions.
  • Ensure that the linter/pre-commit hook is run, I see that it makes changes to formatting when I run it on my side. Done.
  • If the intention was to replace all calls to ´is_finn_op´ with ´is_custom_op´ there is still calls to the former in ´test_channelslast.py´. Done
  • Ensure that all tests pass. Done, there were some tests accessing the op list in the registry in a way that is not compatible with the new registry.

maltanar and others added 30 commits September 25, 2025 11:31
grabs CustomOp instance with the right opset version from protobuf
imported opsets
Replace manual dict registration with automatic class discovery and version resolution.
CustomOps now declare op_version attribute and are auto-registered via module imports.

- Add op_version attribute to all CustomOp base and subclasses
- Implement nested registry structure (domain->op_type->version)
- Support "since version" semantics for automatic version resolution
- Replace is_finn_op() with is_custom_op() from registry
- Add registry API (add_op_to_domain, get_supported_versions, is_custom_op)
- Simplify domain __init__.py files to use __all__ exports
- Use lazy loading via __all__ for performance
…e inference

CustomOp version is now determined solely by _vN class name suffix:
- No suffix defaults to version 1
- _vN suffix indicates version N
- Removes redundant op_version class attribute across all CustomOp implementations
- Updates registry to extract version from class names only
- Adds backward compatibility for legacy custom_op dict pattern
- Simplifies version discovery logic with custom_op dict fallback
@maltanar maltanar changed the title Fix/op version main merge yaman changes Op versioning for new registry system Nov 19, 2025
@maltanar maltanar marked this pull request as ready for review November 19, 2025 09:22
@maltanar maltanar merged commit 0bc01a4 into main Nov 19, 2025
2 of 5 checks passed
@maltanar maltanar deleted the fix/op_version-main-merge-yaman-changes branch November 19, 2025 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants