From ee7e86910691204a3616a3a618186f6cf42ca256 Mon Sep 17 00:00:00 2001 From: liangzhibo Date: Wed, 24 Aug 2022 14:35:45 +0800 Subject: [PATCH] fix hasattr problem --- .jenkins/check/config/filter_pylint.txt | 1 + .../operator/ops_front_infer_function.cc | 7 -- .../optimizer/irpass/symbol_resolver.cc | 22 +++++-- mindspore/ccsrc/pipeline/jit/parse/parse.cc | 64 ++++++++++++++----- mindspore/ccsrc/pipeline/jit/parse/parse.h | 5 ++ mindspore/ccsrc/pipeline/jit/parse/resolve.cc | 3 - .../pipeline/jit/static_analysis/prim.cc | 5 +- mindspore/ccsrc/pybind_api/ir/dtype_py.cc | 1 + .../python/mindspore/_extends/parse/parser.py | 2 +- .../_extends/parse/standard_method.py | 5 +- mindspore/python/mindspore/common/__init__.py | 5 +- mindspore/python/mindspore/common/dtype.py | 4 +- ...ed_name_or_unsupported_builtin_function.py | 10 +-- .../ut/python/python_builtin/test_getattr.py | 37 ++++++----- .../ut/python/python_builtin/test_hasattr.py | 30 +++++++++ 15 files changed, 132 insertions(+), 69 deletions(-) diff --git a/.jenkins/check/config/filter_pylint.txt b/.jenkins/check/config/filter_pylint.txt index 05c62aafd18..d0eaf8766bd 100644 --- a/.jenkins/check/config/filter_pylint.txt +++ b/.jenkins/check/config/filter_pylint.txt @@ -61,6 +61,7 @@ "mindspore/mindspore/python/mindspore/ops/functional.py" "wildcard-import" "mindspore/mindspore/python/mindspore/ops/functional.py" "unused-wildcard-import" "mindspore/mindspore/python/mindspore/_extends/parse/standard_method.py" "redefined-builtin" +"mindspore/mindspore/python/mindspore/_extends/parse/standard_method.py" "protected-access" "mindspore/mindspore/python/mindspore/common/tensor.py" "redefined-builtin" "mindspore/mindspore/python/mindspore/ops/function/array_func.py" "redefined-builtin" "mindspore/mindspore/python/mindspore/ops/operations/array_ops.py" "redefined-builtin" diff --git a/mindspore/ccsrc/frontend/operator/ops_front_infer_function.cc b/mindspore/ccsrc/frontend/operator/ops_front_infer_function.cc index b375c10689c..3282febc350 100644 --- a/mindspore/ccsrc/frontend/operator/ops_front_infer_function.cc +++ b/mindspore/ccsrc/frontend/operator/ops_front_infer_function.cc @@ -313,13 +313,6 @@ py::object GetMsClassPyObj(const abstract::PartialAbstractClosurePtr &ms_class_a << "got: " << ms_class_args.size() << "."; } auto first_arg = ms_class_args[0]; - MS_EXCEPTION_IF_NULL(first_arg); - auto arg_type = first_arg->BuildType(); - auto arg_type_id = arg_type->type_id(); - if (arg_type_id != kObjectTypeClass) { - MS_LOG(EXCEPTION) << "When the first input to IsInstance is PartialAbstractClosure, its first arg should be of " - << "type kObjectTypeClass but got: " << TypeIdToString(arg_type_id) << "."; - } auto class_value = first_arg->BuildValue(); MS_EXCEPTION_IF_NULL(class_value); return ValueToPyData(class_value); diff --git a/mindspore/ccsrc/frontend/optimizer/irpass/symbol_resolver.cc b/mindspore/ccsrc/frontend/optimizer/irpass/symbol_resolver.cc index 3d148eb1e62..227d670333b 100644 --- a/mindspore/ccsrc/frontend/optimizer/irpass/symbol_resolver.cc +++ b/mindspore/ccsrc/frontend/optimizer/irpass/symbol_resolver.cc @@ -45,17 +45,19 @@ AnfNodePtr Resolver::operator()(const OptimizerPtr &optimizer, const AnfNodePtr auto name_space = GetValueNode(object_node); auto attr_str = GetValue(GetValueNode(attr_node)); parse::SymbolPtr symbol = std::make_shared(attr_str); - return parse::ResolveSymbol(optimizer->manager(), name_space, symbol, node); + auto ret = parse::ResolveSymbol(optimizer->manager(), name_space, symbol, node); + // If object has no attribute, ret will be null. The attribute may not be used. + // Let getattr be resolved in static_analysis. + if (IsValueNode(ret)) { + return nullptr; + } + return ret; } // {prim::kPrimGetAttr, MsClassObject, attr} if (IsValueNode(object_node)) { auto ms_class = GetValueNode(object_node)->obj(); auto attr_str = GetValue(GetValueNode(attr_node)); - auto new_node = parse::ResolveMsClassWithAttr(optimizer->manager(), ms_class, attr_str, node); - if (new_node == nullptr || IsValueNode(new_node)) { - MS_EXCEPTION(AttributeError) << py::str(ms_class) << " object has no attribute: " << attr_str << "."; - } - return new_node; + return parse::ResolveMsClassWithAttr(optimizer->manager(), ms_class, attr_str, node); } // {prim::kPrimGetAttr, bool, attr} if (IsValueNode(object_node)) { @@ -68,7 +70,13 @@ AnfNodePtr Resolver::operator()(const OptimizerPtr &optimizer, const AnfNodePtr auto name_space = GetValueNode(ns_node.GetNode(node)); auto symbol = GetValueNode(sym_node.GetNode(node)); auto manager = optimizer->manager(); - return parse::ResolveSymbol(manager, name_space, symbol, node); + auto ret = parse::ResolveSymbol(manager, name_space, symbol, node); + // If object has no attribute, ret will be null. The attribute may not be used. + // Let getattr be resolved in static_analysis. + if (IsValueNode(ret)) { + return nullptr; + } + return ret; }; // {prim::kPrimGetAttr, object, attr} diff --git a/mindspore/ccsrc/pipeline/jit/parse/parse.cc b/mindspore/ccsrc/pipeline/jit/parse/parse.cc index f110decdb6a..9691caee190 100644 --- a/mindspore/ccsrc/pipeline/jit/parse/parse.cc +++ b/mindspore/ccsrc/pipeline/jit/parse/parse.cc @@ -1329,6 +1329,42 @@ AnfNodePtr Parser::ProcessAttributeWithClassMember(const FunctionBlockPtr &block return var_node; } +AnfNodePtr Parser::ParseMsTensor(const FunctionBlockPtr &block, const py::object &node, const py::object &value_body, + const AnfNodePtr &value_node) { + if (py::hasattr(value_body, "id")) { + std::string module_name = py::cast(python_adapter::GetPyObjAttr(value_body, "id")); + py::dict global_dict = const_cast(block->global_py_params()); + if (global_dict.contains(module_name)) { + py::object module_obj = global_dict[py::str(module_name)]; + std::string module_str = py::cast(py::str(module_obj)); + bool the_module_is_mindspore = module_str.find("module 'mindspore'") != std::string::npos; + if (the_module_is_mindspore) { + std::string script_text = py::cast(ast()->GetAstNodeText(node)); + AnfNodePtr interpret_node = MakeInterpretNode(block, value_node, script_text); + interpret_node->set_interpret(true); + interpret_node->set_interpret_internal_type(true); + return interpret_node; + } + } + } + return nullptr; +} + +AnfNodePtr Parser::ParseNull(const FunctionBlockPtr &block, const py::object &value_body) { + if (py::hasattr(value_body, "id")) { + std::string module_name = py::cast(python_adapter::GetPyObjAttr(value_body, "id")); + py::dict global_dict = const_cast(block->global_py_params()); + if (global_dict.contains(module_name)) { + py::object module_obj = global_dict[py::str(module_name)]; + std::string module_str = py::cast(py::str(module_obj)); + if (module_str.find("module 'mindspore.common.dtype'") != std::string::npos) { + return NewValueNode(std::make_shared()); + } + } + } + return nullptr; +} + // Process call attributes of class type define, eg: x.y() AnfNodePtr Parser::ParseAttribute(const FunctionBlockPtr &block, const py::object &node) { MS_LOG(DEBUG) << "Process ast Attribute"; @@ -1356,24 +1392,18 @@ AnfNodePtr Parser::ParseAttribute(const FunctionBlockPtr &block, const py::objec MS_LOG(DEBUG) << "Attr = " << attr_str; // The fallback feature is enabled in default. static const auto use_fallback = (support_fallback() != "0"); - // Process xxx.Tensor() and xxx is mindspore. Check it with global dict. if (use_fallback && attr_str == "Tensor") { - // Check the module is mindspore - if (py::hasattr(value_body, "id")) { - std::string module_name = py::cast(python_adapter::GetPyObjAttr(value_body, "id")); - py::dict global_dict = const_cast(block->global_py_params()); - if (global_dict.contains(module_name)) { - py::object module_obj = global_dict[py::str(module_name)]; - std::string module_str = py::cast(py::str(module_obj)); - bool the_module_is_mindspore = module_str.find("module 'mindspore'") != std::string::npos; - if (the_module_is_mindspore) { - std::string script_text = py::cast(ast()->GetAstNodeText(node)); - AnfNodePtr interpret_node = MakeInterpretNode(block, value_node, script_text); - interpret_node->set_interpret(true); - interpret_node->set_interpret_internal_type(true); - return interpret_node; - } - } + // Process xxx.Tensor() and xxx is mindspore. + auto ret = ParseMsTensor(block, node, value_body, value_node); + if (ret != nullptr) { + return ret; + } + } + if (attr_str == "_null") { + // For stype._null, return TypeNull value node directly. + auto ret = ParseNull(block, value_body); + if (ret != nullptr) { + return ret; } } AnfNodePtr attr_node = nullptr; diff --git a/mindspore/ccsrc/pipeline/jit/parse/parse.h b/mindspore/ccsrc/pipeline/jit/parse/parse.h index 328e3cd93d8..82da2163195 100644 --- a/mindspore/ccsrc/pipeline/jit/parse/parse.h +++ b/mindspore/ccsrc/pipeline/jit/parse/parse.h @@ -188,6 +188,11 @@ class Parser { AnfNodePtr ParseIfExp(const FunctionBlockPtr &block, const py::object &node); // Process class type define AnfNodePtr ParseAttribute(const FunctionBlockPtr &block, const py::object &node); + // Process ms Tensor + AnfNodePtr ParseMsTensor(const FunctionBlockPtr &block, const py::object &node, const py::object &value_body, + const AnfNodePtr &value_node); + // Process dtype._null + AnfNodePtr ParseNull(const FunctionBlockPtr &block, const py::object &value_body); // Process a compare expression AnfNodePtr ParseCompare(const FunctionBlockPtr &block, const py::object &node); // Process a bool operation diff --git a/mindspore/ccsrc/pipeline/jit/parse/resolve.cc b/mindspore/ccsrc/pipeline/jit/parse/resolve.cc index 8877129d6a2..376d2aef745 100644 --- a/mindspore/ccsrc/pipeline/jit/parse/resolve.cc +++ b/mindspore/ccsrc/pipeline/jit/parse/resolve.cc @@ -647,9 +647,6 @@ AnfNodePtr ResolveMsClassWithAttr(const FuncGraphManagerPtr &manager, const py:: return nullptr; } py::object attr_obj = py::getattr(cls_obj, common::SafeCStr(attr)); - if (py::isinstance(attr_obj)) { - return nullptr; - } AnfNodePtr res_node = ResolveObjectAndAddToManager(manager, attr_obj, get_attr_node); TraceManager::ClearParseOrResolveDebugInfo(); return res_node; diff --git a/mindspore/ccsrc/pipeline/jit/static_analysis/prim.cc b/mindspore/ccsrc/pipeline/jit/static_analysis/prim.cc index 335843eb9cf..918ad52d522 100644 --- a/mindspore/ccsrc/pipeline/jit/static_analysis/prim.cc +++ b/mindspore/ccsrc/pipeline/jit/static_analysis/prim.cc @@ -1428,7 +1428,8 @@ EvalResultPtr GetEvaluatedValueForNameSpaceString(const AbstractBasePtrList &arg if (new_node == nullptr) { MS_LOG(EXCEPTION) << "Resolve node failed"; } - if (IsValueNode(new_node)) { + + if (IsValueNode(new_node)) { // Do not find the attribute. constexpr auto max_args_len = 3; bool has_default = (args_spec_list.size() == max_args_len); @@ -1513,7 +1514,7 @@ EvalResultPtr GetEvaluatedValueForMsClassAttrOrMethod(const AbstractBasePtrList FuncGraphPtr func_graph = out_node->func_graph(); // If the attribute is not found and the default is not set, AttributeError will be raised. auto new_node = parse::ResolveMsClassWithAttr(func_graph->manager(), ms_class->obj(), item_name, out_node); - if (new_node == nullptr || IsValueNode(new_node)) { + if (new_node == nullptr) { constexpr auto max_args_len = 3; bool has_default = (args_spec_list.size() == max_args_len); if (!has_default) { diff --git a/mindspore/ccsrc/pybind_api/ir/dtype_py.cc b/mindspore/ccsrc/pybind_api/ir/dtype_py.cc index 092702254d6..c5503d73ac4 100644 --- a/mindspore/ccsrc/pybind_api/ir/dtype_py.cc +++ b/mindspore/ccsrc/pybind_api/ir/dtype_py.cc @@ -177,5 +177,6 @@ REGISTER_PYBIND_DEFINE( (void)py::class_>(m_sub, "Slice").def(py::init()); (void)py::class_>(m_sub, "TypeEllipsis").def(py::init()); (void)py::class_>(m_sub, "TypeMsClassType").def(py::init()); + (void)py::class_>(m_sub, "TypeNull").def(py::init()); })); } // namespace mindspore diff --git a/mindspore/python/mindspore/_extends/parse/parser.py b/mindspore/python/mindspore/_extends/parse/parser.py index e8020936b17..4774ac5f747 100644 --- a/mindspore/python/mindspore/_extends/parse/parser.py +++ b/mindspore/python/mindspore/_extends/parse/parser.py @@ -244,7 +244,7 @@ def resolve_symbol(namespace, symbol): except Exception as e: if isinstance(e, NotImplementedError): raise e - resolve_ = None + resolve_ = mstype._null logger.debug("Resolve exception occurred, value: %r", e) logger.debug("Resolve type is invalid, namespace: %s, symbol: %s", namespace.__str__(), symbol) diff --git a/mindspore/python/mindspore/_extends/parse/standard_method.py b/mindspore/python/mindspore/_extends/parse/standard_method.py index 8eb64a029a4..1583dc5288c 100644 --- a/mindspore/python/mindspore/_extends/parse/standard_method.py +++ b/mindspore/python/mindspore/_extends/parse/standard_method.py @@ -35,6 +35,7 @@ from ...ops.operations._inner_ops import Format from ...ops.operations import _csr_ops from ...ops.primitive import constexpr from ...ops.function.sparse_func import sparse_add +from ...common import dtype as mstype __all__ = ['MultitypeFuncGraph', 'env_get', 'hyper_add', 'zeros_like', 'ones_like'] @@ -271,8 +272,8 @@ def hasattr(x, attr): # pylint: disable=redefined-builtin Returns: Boolean value, indicates whether the object x has attribute attr. """ - out = getattr(x, attr, None) - return out is not None + out = getattr(x, attr, mstype._null) + return not isinstance(out, mstype._null_type) def astype(x, dtype, copy=True): # pylint: disable=redefined-outer-name diff --git a/mindspore/python/mindspore/common/__init__.py b/mindspore/python/mindspore/common/__init__.py index e0186eee366..b134f304066 100644 --- a/mindspore/python/mindspore/common/__init__.py +++ b/mindspore/python/mindspore/common/__init__.py @@ -20,7 +20,7 @@ from mindspore.common.dtype import Type, int8, byte, int16, short, int32, intc, uint8, ubyte, uint16, ushort, uint32, uintc, uint64, uintp, float16, half, \ float32, single, float64, double, bool_, float_, list_, tuple_, int_, \ uint, number, tensor, string, type_none, tensor_type, Int, \ - complex64, complex128, dtype_to_nptype, issubclass_, \ + complex64, complex128, dtype_to_nptype, issubclass_, _null, _null_type, \ dtype_to_pytype, pytype_to_dtype, get_py_obj_dtype from mindspore.common.dump import set_dump from mindspore.common.parameter import Parameter, ParameterTuple @@ -47,8 +47,9 @@ __all__ = [ "int_", "uint", "number", "tensor", "string", "type_none", + "_null", "tensor_type", - "Type", "Int", + "Type", "Int", "_null_type", "complex64", "complex128", # __method__ from dtype "dtype_to_nptype", "issubclass_", "dtype_to_pytype", diff --git a/mindspore/python/mindspore/common/dtype.py b/mindspore/python/mindspore/common/dtype.py index 4b4fc7a9c7a..22f62ca02fd 100644 --- a/mindspore/python/mindspore/common/dtype.py +++ b/mindspore/python/mindspore/common/dtype.py @@ -40,7 +40,7 @@ __dtype__ = [ "int_", "uint", "number", "tensor", "string", "type_none", - "tensor_type", + "tensor_type", "_null", "Type", "Int", "complex64", "complex128" ] @@ -92,6 +92,7 @@ string = typing.String() list_ = typing.List() tuple_ = typing.Tuple() type_none = typing.TypeNone() +_null = typing.TypeNull() tensor = typing.TensorType() index_slices = typing.RowTensorType() @@ -122,6 +123,7 @@ tensor_type = typing.TensorType csr_tensor_type = typing.CSRTensorType anything_type = typing.TypeAnything ref_type = typing.RefType +_null_type = typing.TypeNull number_type = (int8, int16, diff --git a/tests/ut/python/pipeline/parse/test_use_undefined_name_or_unsupported_builtin_function.py b/tests/ut/python/pipeline/parse/test_use_undefined_name_or_unsupported_builtin_function.py index e783357f576..3ee8d24378b 100644 --- a/tests/ut/python/pipeline/parse/test_use_undefined_name_or_unsupported_builtin_function.py +++ b/tests/ut/python/pipeline/parse/test_use_undefined_name_or_unsupported_builtin_function.py @@ -292,12 +292,9 @@ def test_call_none_in_if(): return ret net = Net() - with pytest.raises(ValueError) as err: + with pytest.raises(AttributeError) as err: net(Tensor([1, 2, 3], mstype.float32)) - assert "not defined" in str(err.value) - assert "tests/ut/python/pipeline/parse/test_use_undefined_name_or_unsupported_builtin_function.py(291)" in \ - str(err.value) - assert "ret = self.func(x)" in str(err.value) + assert "has no attribute" in str(err.value) def test_insert_defined_var(): @@ -333,9 +330,6 @@ def test_insert_defined_var_compute(): @pytest.mark.skip(reason='Not support in graph jit fallback feature yet') def test_call_unsupported_builtin_function_in_while(): class Net(nn.Cell): - def __init__(self): - super(Net, self).__init__() - def construct(self, x, y): ret = 0 number = 5 diff --git a/tests/ut/python/python_builtin/test_getattr.py b/tests/ut/python/python_builtin/test_getattr.py index feb07e1ae65..f5779aec6e6 100644 --- a/tests/ut/python/python_builtin/test_getattr.py +++ b/tests/ut/python/python_builtin/test_getattr.py @@ -523,7 +523,7 @@ def test_getattr_ms_class_with_default(): return getattr(ms_obj, "none", 10) out = foo() - assert out == 10 + assert out is None def test_getattr_ms_class_with_concate_attr_and_default(): @@ -565,24 +565,6 @@ def test_getattr_ms_class_with_wrong_attr(): assert "object has no attribute" in str(err.value) -def test_getattr_ms_class_with_wrong_attr_2(): - """ - Feature: Syntax getattr. - Description: Graph syntax getattr support list input. - Expectation: AttributeError. - """ - ms_obj = MSClass1() - - @ms_function - def foo(): - abs_func = getattr(ms_obj, "none") - return abs_func - - with pytest.raises(AttributeError) as err: - foo() - assert "object has no attribute" in str(err.value) - - class Net(nn.Cell): def __init__(self): @@ -591,6 +573,7 @@ class Net(nn.Cell): self.a1 = Tensor([1]) self.a2 = Tensor([2]) self.a3 = Tensor([3]) + self.none = None def construct(self): return self.a0 @@ -612,6 +595,22 @@ def test_getattr_cell_obj(): assert out == 0 +def test_getattr_cell_obj_2(): + """ + Feature: Syntax getattr. + Description: Graph syntax getattr support cell object input. + Expectation: No exception. + """ + cell_obj = Net() + + @ms_function + def foo(): + return getattr(cell_obj, "none") + + out = foo() + assert out is None + + def test_getattr_cell_obj_concate_input(): """ Feature: Syntax getattr. diff --git a/tests/ut/python/python_builtin/test_hasattr.py b/tests/ut/python/python_builtin/test_hasattr.py index 5c4e3357d9b..73a64124981 100644 --- a/tests/ut/python/python_builtin/test_hasattr.py +++ b/tests/ut/python/python_builtin/test_hasattr.py @@ -183,6 +183,21 @@ def test_hasattr_ms_class_2(): def foo(): return hasattr(ms_obj, "none") + assert foo() + + +def test_hasattr_ms_class_3(): + """ + Feature: Syntax hasattr. + Description: Graph syntax hasattr support ms_class input. + Expectation: No exception. + """ + ms_obj = MSClass1() + + @ms_function + def foo(): + return hasattr(ms_obj, "none2") + assert not foo() @@ -249,6 +264,21 @@ def test_hasattr_cell_obj_2(): def foo(): return hasattr(cell_obj, "none") + assert foo() + + +def test_hasattr_cell_obj_3(): + """ + Feature: Syntax hasattr. + Description: Graph syntax hasattr support cell object input. + Expectation: No exception. + """ + cell_obj = Net() + + @ms_function + def foo(): + return hasattr(cell_obj, "none2") + assert not foo()