Setting node attributes with wrongly typed values causes bad behaviour

[bwintermann]

Prerequisites

Main branch, latest commit (279f9c3). Freshly setup venv with onnx, qonnx and dependencies.

Quick summary

Setting a node attribute with the wrong type causes bad behaviour.

Details

When setting a node attribute in a CustomOp instance using set_nodeattr to the wrong type of value, and consequently requesting it using get_nodeattr this yields the wrong value. I encountered this behaviour for an int-attribute that I accidentally assigned a float to, but also tested it using a string-attribute that I tried assigning an int to.

Steps to Reproduce

1. Set up a fresh virtual environment
2. Install onnx, qonnx and dependencies
3. Run the following code

from qonnx.custom_op.base import CustomOp
from onnx import helper

class MyCustomOp(CustomOp):
    def __init__(self, onnx_node, onnx_opset_version=...):
        super().__init__(onnx_node, onnx_opset_version)
            
    def get_nodeattr_types(self):
        return {
            "my_attribute": ("i", False, -1)
        }

    def execute_node(self, context, graph):
        pass

    def infer_node_datatype(self, model):
        pass

    def make_shape_compatible_op(self, model):
        pass

    def verify_node(self):
        pass

node = helper.make_node("myOpType", [], [])
myCustomOp = MyCustomOp(node, 13)

print(myCustomOp.get_nodeattr("my_attribute"))
myCustomOp.set_nodeattr("my_attribute", 2.0)
print(myCustomOp.get_nodeattr("my_attribute"))
myCustomOp.set_nodeattr("my_attribute", 2)
print(myCustomOp.get_nodeattr("my_attribute"))

This would print

-1
0
2

The first value being the correct default, the second value appearing after using the wrong type to set the attribute and the third value after inserting a value of the correct type.

Fix

When setting an attribute to a value of the wrong type I would either propose raising an exception, ignoring the assignment, throwing a warning or trying to cast the value. Currently the operation happens quietly, and when retrieving a wrong value is found.

[maltanar]

Yes, this can cause all sorts of havoc. Ideally there should be type checking as part of CustomOp.set_nodeattr.