Namespace based customop registration #204
Conversation
Replace dictionary-based registration with module namespace lookup: - Direct attribute lookup in module namespace (preferred) - Legacy custom_op dictionary support (backward compatibility) - Domain aliasing (e.g., onnx.brevitas -> qonnx.custom_op.general) - Runtime op registration via add_op_to_domain() - No expensive fallbacks (removed case-insensitive matching, full module scan) - Clear, actionable error messages Benefits: - Better IDE support with direct imports - Cleaner, more Pythonic API - O(1) lookup performance - Predictable behavior - External packages work via domain-based imports (e.g., finn.custom_op.fpgadataflow)
|
How are these ops in the custom domain suppose to execute? However, the function I also tested the code on chisel4ml, with a custom defined in the chisel4ml.preprocess namespace. I then called Am I missing something? should I be calling something else? Honestly, I just don't see how this can execute. |
| Example: | ||
| add_op_to_domain("qonnx.custom_op.general", "TestOp", TestOp) | ||
| """ | ||
| if not inspect.isclass(op_class) or not issubclass(op_class, CustomOp): |
There was a problem hiding this comment.
This logic is not correct. It evaluates to True even if op_class is not subclass of CustomOp. This code will evaluate to True on anything, even if op_class is a float.
Regardless of that, I do not see the point of using inspect here. If the intention is to prevent the user from using an object instance here, issubclass will raise a TypeError. Which is a more appropriate error than ValueError.
There was a problem hiding this comment.
This check is from an old iteration where it cached all CustomOps at a given domain on first access, this avoided illegal comparisons. Thanks for catching this, will remove in next commit.
| # Strategy 1: Direct namespace lookup (preferred) | ||
| if hasattr(module, op_type): | ||
| obj = getattr(module, op_type) | ||
| if inspect.isclass(obj) and issubclass(obj, CustomOp): |
There was a problem hiding this comment.
is obj a class? judging from the code bellow it is a class not an object. This makes the variable name obj somewhat misleading. Also the same argument about inspect holds here.
2 participants
Summary
This PR enhances the CustomOp registration system to support Python module namespace lookups alongside the existing dictionary-based approach. The new system provides better IDE support and a more Pythonic API while maintaining full backward compatibility.
Key Changes
Core Registry (
src/qonnx/custom_op/registry.py)add_op_to_domain()for runtime op registrationModule Structure Updates
channels_lastandgeneralmodules automatically register themcustom_opdictionaries for backward compatibilityQuantimport in general moduleBenefits
Migration Path
The system automatically falls back to legacy dictionaries, so no immediate changes are required. New CustomOps can be registered by:
<domain_path>/__init__.pyadd_op_to_domain()for runtime registrationcustom_opdictionaries (legacy)