[PyOV] Dynamic attributes for Python based operators (#21256)

src/bindings/python/src/openvino/runtime/opset2/ops.py

@@ -7,6 +7,7 @@ from typing import Callable, Iterable, List, Optional, Set, Union
 
 import numpy as np
 from functools import partial
+import warnings
 
 from openvino.runtime import Node, Shape
 from openvino.runtime.op import Constant, Parameter
@@ -129,31 +130,64 @@ def reorg_yolo(input: Node, stride: List[int], name: Optional[str] = None) -> No
 def roi_pooling(
     input: NodeInput,
     coords: NodeInput,
-    output_size: TensorShape,
-    spatial_scale: NumericData,
-    method: str,
+    output_roi: Optional[TensorShape] = None,
+    spatial_scale: Optional[NumericData] = None,
+    method: str = "max",
     name: Optional[str] = None,
+    *,
+    output_size: Optional[TensorShape] = None,
 ) -> Node:
     """Return a node which produces an ROIPooling operation.
 
     :param input:          Input feature map `{N, C, ...}`.
     :param coords:         Coordinates of bounding boxes.
-    :param output_size:    Height/Width of ROI output features (shape).
+    :param output_roi:     Height/Width of ROI output features (shape).
     :param spatial_scale:  Ratio of input feature map over input image size (float).
-    :param method:         Method of pooling - string: "max" or "bilinear".
+    :param method:         Method of pooling - string: "max" or "bilinear". Default: "max"
+    :param output_size:    (DEPRECATED!) Height/Width of ROI output features (shape).
+                           Will override `output_roi` if used and change behavior of the operator.
     :return:               ROIPooling node.
     """
+    # Allow either one of these attributes to be passed.
+    if output_roi is None and output_size is None:
+        raise AttributeError("One of the following arguments must be defined: `output_roi`, `output_size`!")
+    # Force checking of spatial_scale.
+    if spatial_scale is None:
+        raise AttributeError("The following arguments must be defined: `spatial_scale`!")
+
+    def _deprecated_output_size_arg(output_roi: Optional[TensorShape], output_size: Optional[TensorShape]) -> Optional[TensorShape]:
+        if output_size is not None:
+            warnings.warn(
+                "`output_size` is deprecated and will be removed in future. "
+                "Value of `output_size` is going to override `output_roi` value and "
+                "`get_output_size` will behave like `get_output_roi` function."
+                "Please use only `output_roi` explicitly.",
+                DeprecationWarning,
+                stacklevel=3,
+            )
+            return output_size
+        return output_roi
+
     method = method.lower()
-    return _get_node_factory_opset2().create(
+    roi_shape = _deprecated_output_size_arg(output_roi, output_size)
+    node = _get_node_factory_opset2().create(
         "ROIPooling",
         as_nodes(input, coords),
         {
-            "output_size": Shape(output_size),
+            "output_size": Shape(roi_shape),
+            "output_roi": Shape(roi_shape),
             "spatial_scale": spatial_scale,
             "method": method,
         },
     )
 
+    # Override behavior when deprecated value was used.
+    if output_size is not None:
+        node.get_output_size = node.get_output_roi
+        node.set_output_size = node.set_output_roi
+
+    return node
+
 
 @nameable_op
 def space_to_batch(

src/bindings/python/src/openvino/runtime/utils/node_factory.py

@@ -45,8 +45,6 @@ class NodeFactory(object):
         """
         if arguments is None and attributes is None:
             node = self.factory.create(op_type_name)
-            node._attr_cache = {}
-            node._attr_cache_valid = False
             return node
 
         if arguments is None and attributes is not None:
@@ -60,37 +58,6 @@ class NodeFactory(object):
         arguments = self._arguments_as_outputs(arguments)
         node = self.factory.create(op_type_name, arguments, attributes)
 
-        # Currently we don't support any attribute getters & setters for TensorIterator node.
-        if node.get_type_name() == "TensorIterator":
-            return node
-
-        """Set getters and setters for each node's attribute.
-            node.get_attribute_name()
-            node.set_attribute_name()
-        For compound (with more than one level of nesting) attributes of form ie.:
-        node.class_member_name.some_metric.attr_name:
-            node.get_some_metric_attr_name()
-            node.set_some_metric_attr_name()
-        Please see test_dyn_attributes.py for more usage examples.
-        """
-        all_attributes = node.get_attributes()
-        for attr_name in all_attributes.keys():
-            setattr(
-                node,
-                self._normalize_attr_name_getter(attr_name),
-                partial(NodeFactory._get_node_attr_value, node, attr_name),
-            )
-            setattr(
-                node,
-                self._normalize_attr_name_setter(attr_name),
-                partial(NodeFactory._set_node_attr_value, node, attr_name),
-            )
-
-        # Setup helper members for caching attribute values.
-        # The cache would be lazily populated at first access attempt.
-        node._attr_cache = {}
-        node._attr_cache_valid = False
-
         return node
 
     def add_extension(self, lib_path: Union[Path, str]) -> None:
@@ -126,62 +93,3 @@ class NodeFactory(object):
             else:
                 outputs.extend(argument.outputs())
         return outputs
-
-    @staticmethod
-    def _normalize_attr_name(attr_name: str, prefix: str) -> str:
-        """Normalize attribute name.
-
-        :param      attr_name:  The attribute name.
-        :param      prefix:     The prefix to attach to attribute name.
-
-        :return:   The modified attribute name.
-        """
-        # Trim first part of the name if there is only one level of attribute hierarchy.
-        if attr_name.count(".") == 1:
-            attr_name = attr_name[attr_name.find(".") + 1:]
-        return prefix + attr_name.replace(".", "_")
-
-    @staticmethod
-    def _get_node_attr_value(node: Node, attr_name: str) -> Any:
-        """Get provided node attribute value.
-
-        :param      node:       The node we retrieve attribute value from.
-        :param      attr_name:  The attribute name.
-
-        :return:   The node attribute value.
-        """
-        if not node._attr_cache_valid:
-            node._attr_cache = node.get_attributes()
-            node._attr_cache_valid = True
-        return node._attr_cache[attr_name]
-
-    @staticmethod
-    def _set_node_attr_value(node: Node, attr_name: str, value: Any) -> None:
-        """Set the node attribute value.
-
-        :param      node:       The node we change attribute value for.
-        :param      attr_name:  The attribute name.
-        :param      value:      The new attribute value.
-        """
-        node.set_attribute(attr_name, value)
-        node._attr_cache[attr_name] = value
-
-    @classmethod
-    def _normalize_attr_name_getter(cls, attr_name: str) -> str:
-        """Normalize atr name to be suitable for getter function name.
-
-        :param      attr_name:  The attribute name to normalize
-
-        :return:   The appropriate getter function name.
-        """
-        return cls._normalize_attr_name(attr_name, "get_")
-
-    @classmethod
-    def _normalize_attr_name_setter(cls, attr_name: str) -> str:
-        """Normalize attribute name to be suitable for setter function name.
-
-        :param      attr_name:  The attribute name to normalize
-
-        :return:   The appropriate setter function name.
-        """
-        return cls._normalize_attr_name(attr_name, "set_")

src/bindings/python/src/pyopenvino/graph/dict_attribute_visitor.hpp

@@ -84,12 +84,12 @@ public:
     void on_adapter(const std::string& name, ov::ValueAccessor<std::vector<float>>& adapter) override;
     void on_adapter(const std::string& name, ov::ValueAccessor<std::vector<double>>& adapter) override;
 
+    bool contains_attribute(const std::string& name) {
+        return m_attributes.contains(name);
+    }
+
     template <typename T>
     T get_attribute(const std::string& name) {
-        OPENVINO_ASSERT(m_attributes.contains(name),
-                        "Couldn't find attribute \"",
-                        name,
-                        "\" in serialized node attribute dictionary.");
         return m_attributes[name.c_str()].cast<T>();
     }
 

src/bindings/python/src/pyopenvino/graph/node.cpp

@@ -8,6 +8,9 @@
 #include <pybind11/stl.h>
 #include <pybind11/stl_bind.h>
 
+#include <regex>
+#include <string>
+
 #include "dict_attribute_visitor.hpp"
 #include "openvino/core/runtime_attribute.hpp"
 #include "openvino/op/add.hpp"
@@ -80,6 +83,53 @@ void regclass_graph_Node(py::module m) {
         }
         return "<" + type_name + ": '" + self.get_friendly_name() + "' (" + shapes_ss.str() + ")>";
     });
+
+    // This function replaces NodeFactory's mechanism previously getting attributes of Nodes.
+    // Python will call this method whenever requested attribute hasn't already been defined,
+    // so all other properties like shape or name are prioritized from bindings itself.
+    // TODO: is it possible to append these attributes to the instance of Node itself
+    //       directly from pybind without returning class itself everytime?
+    node.def("__getattr__", [](const std::shared_ptr<ov::Node>& self, const std::string& name) {
+        // TODO: is it possible to cache serializer and regex?
+        util::DictAttributeSerializer dict_serializer(self);
+
+        // Look if there is a "get/set_*" pattern at the start of a string.
+        // It means attribute was called as a function.
+        std::smatch regex_match;
+        std::regex look_for("get_|set_");
+
+        if (std::regex_search(name, regex_match, look_for, std::regex_constants::match_continuous)) {
+            // Strip prefix from the name and perform operation.
+            auto stripped_name = regex_match.suffix().str();
+            if (regex_match.str() == "get_") {
+                if (dict_serializer.contains_attribute(stripped_name)) {
+                    return py::cpp_function([self, stripped_name]() {
+                        util::DictAttributeSerializer dict_serializer(self);
+                        return dict_serializer.get_attribute<py::object>(stripped_name);
+                    });
+                }
+                // Throw error with original name if stripped set_attribute was not found:
+                throw py::attribute_error("'openvino.runtime.Node' object has no attribute '" + name + "'");
+            } else {  // regex_match is equal to "set_"
+                if (dict_serializer.contains_attribute(stripped_name)) {
+                    return py::cpp_function([self, stripped_name](py::object& value) {
+                        py::dict attr_dict;
+                        attr_dict[stripped_name.c_str()] = value;
+                        std::unordered_map<std::string, std::shared_ptr<ov::op::util::Variable>> variables;
+                        util::DictAttributeDeserializer dict_deserializer(attr_dict, variables);
+                        self->visit_attributes(dict_deserializer);
+                        return;
+                    });
+                }
+                // Throw error with original name if stripped set_attribute was not found:
+                throw py::attribute_error("'openvino.runtime.Node' object has no attribute '" + name + "'");
+            }
+        }
+
+        // If nothing was found raise AttributeError:
+        throw py::attribute_error("'openvino.runtime.Node' object has no attribute '" + name + "'");
+    });
+
     node.def(
         "evaluate",
         [](const ov::Node& self,

src/bindings/python/tests/test_graph/test_create_op.py

@@ -804,9 +804,38 @@ def test_roi_pooling():
     node = ov.roi_pooling(inputs, coords, [6, 6], 0.0625, "Max")
 
     assert node.get_type_name() == "ROIPooling"
+    assert node.get_output_roi() == [6, 6]
+    assert list(node.get_output_shape(0)) == [150, 3, 6, 6]
+    assert node.get_output_element_type(0) == Type.f32
+    node.set_output_roi([2, 1])
+    assert node.get_output_roi() == [2, 1]
+
+
+def test_roi_pooling_deprecation():
+    inputs = ov.parameter([2, 3, 4, 5], dtype=np.float32)
+    coords = ov.parameter([150, 5], dtype=np.float32)
+
+    with pytest.raises(AttributeError) as e:
+        _ = ov.roi_pooling(inputs, coords=coords, spatial_scale=0.0625, method="Max")
+    assert "One of the following arguments must be defined: `output_roi`, `output_size`!" in str(e.value)
+
+    with pytest.raises(AttributeError) as e:
+        _ = ov.roi_pooling(inputs, coords=coords, output_roi=[6, 6])
+    assert "The following arguments must be defined: `spatial_scale`!" in str(e.value)
+
+    with pytest.warns(DeprecationWarning) as w:
+        node = ov.roi_pooling(inputs, coords=coords, output_size=[6, 6], spatial_scale=0.0625, method="Max")
+    assert issubclass(w[0].category, DeprecationWarning)
+    assert "`output_size` is deprecated and will be removed in future" in str(w[0].message)
+
+    assert node.get_type_name() == "ROIPooling"
+    assert node.get_output_roi() == [6, 6]
     assert node.get_output_size() == [6, 6]
     assert list(node.get_output_shape(0)) == [150, 3, 6, 6]
     assert node.get_output_element_type(0) == Type.f32
+    node.set_output_size([2, 1])  # the same as: node.set_output_roi([2, 1])
+    assert node.get_output_roi() == [2, 1]
+    assert node.get_output_size() == [2, 1]
 
 
 @pytest.mark.parametrize(
@@ -2161,6 +2190,7 @@ def test_interpolate_opset10(dtype, expected_shape, shape_calculation_mode):
     assert node.get_type_name() == "Interpolate"
     assert node.get_output_size() == 1
     assert list(node.get_output_shape(0)) == expected_shape
+    assert node.get_shape_calculation_mode() == shape_calculation_mode
 
 
 @pytest.mark.parametrize(

src/bindings/python/tests/test_graph/test_dyn_attributes.py

@@ -97,16 +97,6 @@ def test_dynamic_set_attribute_value(int_dtype, fp_dtype):
     assert node.get_framework() == "OpenVINO"
 
 
-def test_dynamic_attr_cache(proposal_node):
-    node = proposal_node
-
-    assert not node._attr_cache_valid
-    node.set_nms_thresh(1.3453678102)
-    assert not node._attr_cache_valid
-    assert np.isclose(node.get_nms_thresh(), np.float64(1.3453678102))
-    assert node._attr_cache_valid
-
-
 def test_dynamic_attr_transitivity(proposal_node):
     node = proposal_node
     node2 = node