From 33338bde10426c5068238d52b0122fe6c2525f10 Mon Sep 17 00:00:00 2001 From: Chris Sidebottom Date: Tue, 7 Sep 2021 15:14:39 +0000 Subject: [PATCH 1/3] Introduce centralised name transformation functions To address some of the concerns raised in https://github.com/apache/tvm/pull/8280 and https://github.com/apache/tvm/pull/8720 I've put together a series of functions to combine together names to re-use between these areas. These are meant to be a starting point to fix up the name generation to use the TVM C conventions and port the interface API header to C++. These functions will also be used for constructing the names in the C Device API (https://github.com/apache/tvm-rfcs/pull/31). --- python/tvm/relay/backend/name_transforms.py | 86 ++++++++++++++++ src/relay/backend/name_transforms.cc | 102 +++++++++++++++++++ src/relay/backend/name_transforms.h | 105 ++++++++++++++++++++ tests/cpp/name_transforms_test.cc | 85 ++++++++++++++++ tests/python/relay/test_name_transforms.py | 96 ++++++++++++++++++ 5 files changed, 474 insertions(+) create mode 100644 python/tvm/relay/backend/name_transforms.py create mode 100644 src/relay/backend/name_transforms.cc create mode 100644 src/relay/backend/name_transforms.h create mode 100644 tests/cpp/name_transforms_test.cc create mode 100644 tests/python/relay/test_name_transforms.py diff --git a/python/tvm/relay/backend/name_transforms.py b/python/tvm/relay/backend/name_transforms.py new file mode 100644 index 000000000000..a7bfa9847edd --- /dev/null +++ b/python/tvm/relay/backend/name_transforms.py @@ -0,0 +1,86 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" +Name transformation functions for use in code generation +""" + +from typing import List, Union + +from . import _backend + + +def to_c_function_style(original_name: str): + """Transform a name to the C function style assuming it is + appropriately constructed using the prefixing functions + + Parameters + ---------- + original_name : str + Original name to transform + """ + return _backend.ToCFunctionStyle(original_name) + + +def to_c_variable_style(original_name: str): + """Transform a name to the C variable style assuming it is + appropriately constructed using the prefixing functions + + Parameters + ---------- + original_name : str + Original name to transform + """ + return _backend.ToCVariableStyle(original_name) + + +def prefix_name(names: Union[List[str], str]): + """Apply TVM-specific prefix to a function name + + Parameters + ---------- + names : Union[List[str], str] + List of names to combine to form a combined name or the name itself + """ + if isinstance(names, str): + names = [names] + + return _backend.PrefixName(names) + + +def prefix_generated_name(names: Union[List[str], str]): + """Apply generated TVM-specific prefix to a function name + + Parameters + ---------- + names : Union[List[str], str] + List of names to combine to form a combined name or the name itself + """ + if isinstance(names, str): + names = [names] + + return _backend.PrefixGeneratedName(names) + + +def sanitise_name(original_name: str): + """Sanitise name for output into compiler artifacts + + Parameters + ---------- + original_name : str + Original name to sanitise + """ + return _backend.SanitiseName(original_name) diff --git a/src/relay/backend/name_transforms.cc b/src/relay/backend/name_transforms.cc new file mode 100644 index 000000000000..f09e6a6346a5 --- /dev/null +++ b/src/relay/backend/name_transforms.cc @@ -0,0 +1,102 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "name_transforms.h" + +#include + +#include +#include + +namespace tvm { +namespace relay { +namespace backend { + +std::string ToCFunctionStyle(const std::string& original_name) { + ICHECK(!original_name.empty()); + + int tvm_prefix_length = 3; + std::string function_name("TVM"); + + bool new_block = true; + for (const char& symbol : original_name.substr(tvm_prefix_length)) { + if (std::isalpha(symbol)) { + if (new_block) { + function_name.push_back(std::toupper(symbol)); + new_block = false; + } else { + function_name.push_back(std::tolower(symbol)); + } + } else if (symbol == '_') { + new_block = true; + } + } + return function_name; +} + +std::string ToCVariableStyle(const std::string& original_name) { + ICHECK(!original_name.empty()); + + std::string variable_name; + variable_name.resize(original_name.size()); + + std::transform(original_name.begin(), original_name.end(), variable_name.begin(), ::tolower); + return variable_name; +} + +std::string CombineNames(const Array& names) { + std::stringstream combine_stream; + ICHECK(!names.empty()); + + for (const String& name : names) { + ICHECK(!name.empty()); + combine_stream << name << "_"; + } + + std::string combined_name = combine_stream.str(); + combined_name.pop_back(); + return combined_name; +} + +std::string SanitiseName(const std::string& name) { + ICHECK(!name.empty()); + + auto multipleSeparators = [](char before, char after) { + return before == '_' && before == after; + }; + auto isNotAlnum = [](char c) { return !std::isalnum(c); }; + std::string sanitised_input = name; + std::replace_if(sanitised_input.begin(), sanitised_input.end(), isNotAlnum, '_'); + + sanitised_input.erase( + std::unique(sanitised_input.begin(), sanitised_input.end(), multipleSeparators), + sanitised_input.end()); + + return sanitised_input; +} + +TVM_REGISTER_GLOBAL("relay.backend.ToCFunctionStyle").set_body_typed(ToCFunctionStyle); +TVM_REGISTER_GLOBAL("relay.backend.ToCVariableStyle").set_body_typed(ToCVariableStyle); +TVM_REGISTER_GLOBAL("relay.backend.PrefixName").set_body_typed(PrefixName); +TVM_REGISTER_GLOBAL("relay.backend.PrefixGeneratedName").set_body_typed(PrefixGeneratedName); +TVM_REGISTER_GLOBAL("relay.backend.SanitiseName").set_body_typed(SanitiseName); + +} // namespace backend +} // namespace relay +} // namespace tvm diff --git a/src/relay/backend/name_transforms.h b/src/relay/backend/name_transforms.h new file mode 100644 index 000000000000..f94b472f88c8 --- /dev/null +++ b/src/relay/backend/name_transforms.h @@ -0,0 +1,105 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/*! + * \file relay/backend/name_transforms.h + * \brief Transformations which are applied on names to generate appropriately named compiler + * artifacts + * + * Example: + * ToCFunctionStyle(PrefixName(CombineNames({"Device", "target", "Invoke"}))) + * // TVMDeviceTargetInvoke + * + * ToCFunctionStyle(PrefixGeneratedName(CombineNames({"model", "Run"}))) + * // TVMGenModelRun + * + * ToCVariableStyle(PrefixName(CombineNames({"Device", "target", "t"}))) + * // tvm_device_target_t + * + * ToCVariableStyle(PrefixGeneratedName(CombineNames({"model", "Devices"}))) + * // tvmgen_model_devices + * + */ + +#include +#include +#include + +#include +#include +#include + +#ifndef TVM_RELAY_BACKEND_NAME_TRANSFORMS_H_ +#define TVM_RELAY_BACKEND_NAME_TRANSFORMS_H_ + +namespace tvm { +namespace relay { +namespace backend { + +/*! + * \brief Transform a name to the C variable style assuming it is + * appropriately constructed using the prefixing functions + * \param original_name Original name + * \return Transformed function in the C function style + */ +std::string ToCFunctionStyle(const std::string& original_name); + +/*! + * \brief Transform a name to the C variable style assuming it is + * appropriately constructed using the prefixing functions + * \param name Original name + * \return Transformed function in the C variable style + */ +std::string ToCVariableStyle(const std::string& original_name); + +/*! + * \brief Combine names together for use as a generated name + * \param names Vector of strings to combine + * \return Combined together names + */ +std::string CombineNames(const Array& names); + +/*! + * \brief Apply TVM-specific prefix to a name + * \param names Vector of names to combine to form a combined name + * \return Name with prefix applied or prefix-only if no name passed + */ +inline std::string PrefixName(const Array& names) { return "TVM_" + CombineNames(names); } + +/*! + * \brief Apply generated TVM-specific prefix to a name + * \param names Vector of names to combine to form a combined name + * \return Name with prefix applied or prefix-only if no name passed + */ +inline std::string PrefixGeneratedName(const Array& names) { + return "TVMGen_" + CombineNames(names); +} + +/*! + * \brief Sanitise name for output into compiler artifacts + * \param name Original name + * \return Sanitised name + */ +std::string SanitiseName(const std::string& name); + +} // namespace backend +} // namespace relay +} // namespace tvm + +#endif // TVM_RELAY_BACKEND_NAME_TRANSFORMS_H_ diff --git a/tests/cpp/name_transforms_test.cc b/tests/cpp/name_transforms_test.cc new file mode 100644 index 000000000000..bcad3f9db091 --- /dev/null +++ b/tests/cpp/name_transforms_test.cc @@ -0,0 +1,85 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "../src/relay/backend/name_transforms.h" + +#include +#include + +using namespace tvm::relay::backend; +using namespace tvm::runtime; + +TEST(NameTransforms, ToCFunctionStyle) { + ASSERT_EQ(ToCFunctionStyle("TVM_Woof"), "TVMWoof"); + ASSERT_EQ(ToCFunctionStyle("TVM_woof"), "TVMWoof"); + ASSERT_EQ(ToCFunctionStyle("TVM_woof_woof"), "TVMWoofWoof"); + ASSERT_EQ(ToCFunctionStyle("TVMGen_woof_woof"), "TVMGenWoofWoof"); + EXPECT_THROW(ToCFunctionStyle(""), InternalError); +} + +TEST(NameTransforms, ToCVariableStyle) { + ASSERT_EQ(ToCVariableStyle("TVM_Woof"), "tvm_woof"); + ASSERT_EQ(ToCVariableStyle("TVM_woof"), "tvm_woof"); + ASSERT_EQ(ToCVariableStyle("TVM_woof_Woof"), "tvm_woof_woof"); + EXPECT_THROW(ToCVariableStyle(""), InternalError); +} + +TEST(NameTransforms, PrefixName) { + ASSERT_EQ(PrefixName({"Woof"}), "TVM_Woof"); + ASSERT_EQ(PrefixName({"woof"}), "TVM_woof"); + ASSERT_EQ(PrefixName({"woof", "moo"}), "TVM_woof_moo"); + EXPECT_THROW(PrefixName({}), InternalError); + EXPECT_THROW(PrefixName({""}), InternalError); +} + +TEST(NameTransforms, PrefixGeneratedName) { + ASSERT_EQ(PrefixGeneratedName({"Woof"}), "TVMGen_Woof"); + ASSERT_EQ(PrefixGeneratedName({"woof"}), "TVMGen_woof"); + ASSERT_EQ(PrefixGeneratedName({"woof", "moo"}), "TVMGen_woof_moo"); + EXPECT_THROW(PrefixGeneratedName({}), InternalError); + EXPECT_THROW(PrefixGeneratedName({""}), InternalError); +} + +TEST(NameTransforms, CombineNames) { + ASSERT_EQ(CombineNames({"woof"}), "woof"); + ASSERT_EQ(CombineNames({"Woof", "woof"}), "Woof_woof"); + ASSERT_EQ(CombineNames({"Woof", "woof", "woof"}), "Woof_woof_woof"); + ASSERT_EQ(CombineNames({"Woof", "moo", "t"}), "Woof_moo_t"); + + EXPECT_THROW(CombineNames({}), InternalError); + EXPECT_THROW(CombineNames({""}), InternalError); + EXPECT_THROW(CombineNames({"Woof", ""}), InternalError); + EXPECT_THROW(CombineNames({"", "Woof"}), InternalError); +} + +TEST(NameTransforms, SanitiseName) { + ASSERT_EQ(SanitiseName("+_+ "), "_"); + ASSERT_EQ(SanitiseName("input+"), "input_"); + ASSERT_EQ(SanitiseName("input-"), "input_"); + ASSERT_EQ(SanitiseName("input++"), "input_"); + ASSERT_EQ(SanitiseName("woof:1"), "woof_1"); + EXPECT_THROW(SanitiseName(""), InternalError); +} + +TEST(NameTransforms, CombinedLogic) { + ASSERT_EQ(ToCFunctionStyle(PrefixName({"Device", "target", "Invoke"})), "TVMDeviceTargetInvoke"); + ASSERT_EQ(ToCFunctionStyle(PrefixGeneratedName({"model", "Run"})), "TVMGenModelRun"); + ASSERT_EQ(ToCVariableStyle(PrefixName({"Device", "target", "t"})), "tvm_device_target_t"); + ASSERT_EQ(ToCVariableStyle(PrefixGeneratedName({"model", "Devices"})), "tvmgen_model_devices"); +} diff --git a/tests/python/relay/test_name_transforms.py b/tests/python/relay/test_name_transforms.py new file mode 100644 index 000000000000..34ea100392f5 --- /dev/null +++ b/tests/python/relay/test_name_transforms.py @@ -0,0 +1,96 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License" you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from tvm import TVMError +from tvm.relay.backend.name_transforms import ( + to_c_function_style, + to_c_variable_style, + prefix_name, + prefix_generated_name, + sanitise_name, +) +import pytest + + +def test_to_c_function_style(): + assert to_c_function_style("TVM_Woof") == "TVMWoof" + assert to_c_function_style("TVM_woof") == "TVMWoof" + assert to_c_function_style("TVM_woof_woof") == "TVMWoofWoof" + assert to_c_function_style("TVMGen_woof_woof") == "TVMGenWoofWoof" + + with pytest.raises(TVMError): + to_c_function_style("") + + +def test_to_c_variable_style(): + assert to_c_variable_style("TVM_Woof") == "tvm_woof" + assert to_c_variable_style("TVM_woof") == "tvm_woof" + assert to_c_variable_style("TVM_woof_Woof") == "tvm_woof_woof" + + with pytest.raises(TVMError): + to_c_variable_style("") + + +def test_prefix_name(): + assert prefix_name("Woof") == "TVM_Woof" + assert prefix_name(["Woof"]) == "TVM_Woof" + assert prefix_name(["woof"]) == "TVM_woof" + assert prefix_name(["woof", "moo"]) == "TVM_woof_moo" + + with pytest.raises(TVMError): + prefix_name("") + with pytest.raises(TVMError): + prefix_name([]) + with pytest.raises(TVMError): + prefix_name([""]) + + +def test_prefix_generated_name(): + assert prefix_generated_name("Woof") == "TVMGen_Woof" + assert prefix_generated_name(["Woof"]) == "TVMGen_Woof" + assert prefix_generated_name(["Woof"]) == "TVMGen_Woof" + assert prefix_generated_name(["woof"]) == "TVMGen_woof" + assert prefix_generated_name(["woof", "moo"]) == "TVMGen_woof_moo" + + with pytest.raises(TVMError): + prefix_generated_name("") + with pytest.raises(TVMError): + prefix_generated_name([]) + with pytest.raises(TVMError): + prefix_generated_name([""]) + + +def test_sanitise_name(): + assert sanitise_name("+_+ ") == "_" + assert sanitise_name("input+") == "input_" + assert sanitise_name("input-") == "input_" + assert sanitise_name("input++") == "input_" + assert sanitise_name("woof:1") == "woof_1" + + with pytest.raises(TVMError): + sanitise_name("") + + +def test_combined_logic(): + assert ( + to_c_function_style(prefix_name(["Device", "target", "Invoke"])) == "TVMDeviceTargetInvoke" + ) + assert to_c_function_style(prefix_generated_name(["model", "Run"])) == "TVMGenModelRun" + assert to_c_variable_style(prefix_name(["Device", "target", "t"])) == "tvm_device_target_t" + assert ( + to_c_variable_style(prefix_generated_name(["model", "Devices"])) == "tvmgen_model_devices" + ) From 52e41d58f22defc8f302c2f496d72a92c2120172 Mon Sep 17 00:00:00 2001 From: Chris Sidebottom Date: Tue, 28 Sep 2021 11:16:20 +0000 Subject: [PATCH 2/3] Improve error handling and error messages --- python/tvm/relay/backend/name_transforms.py | 24 +++++++++++++++------ src/relay/backend/name_transforms.cc | 12 ++++++----- tests/cpp/name_transforms_test.cc | 2 ++ tests/python/relay/test_name_transforms.py | 24 +++++++++++++-------- 4 files changed, 42 insertions(+), 20 deletions(-) diff --git a/python/tvm/relay/backend/name_transforms.py b/python/tvm/relay/backend/name_transforms.py index a7bfa9847edd..2107485bd1ae 100644 --- a/python/tvm/relay/backend/name_transforms.py +++ b/python/tvm/relay/backend/name_transforms.py @@ -20,6 +20,7 @@ from typing import List, Union +from tvm import TVMError from . import _backend @@ -47,6 +48,21 @@ def to_c_variable_style(original_name: str): return _backend.ToCVariableStyle(original_name) +def _preprocess_names(names: Union[List[str], str]): + """Preprocesses name strings into format for C++ functions + + Parameters + ---------- + names : Union[List[str], str] + List of names to combine to form a combined name or the name itself + """ + if isinstance(names, str): + if names == "": + raise TVMError("Name is empty") + return [names] + return names + + def prefix_name(names: Union[List[str], str]): """Apply TVM-specific prefix to a function name @@ -55,10 +71,8 @@ def prefix_name(names: Union[List[str], str]): names : Union[List[str], str] List of names to combine to form a combined name or the name itself """ - if isinstance(names, str): - names = [names] - return _backend.PrefixName(names) + return _backend.PrefixName(_preprocess_names(names)) def prefix_generated_name(names: Union[List[str], str]): @@ -69,10 +83,8 @@ def prefix_generated_name(names: Union[List[str], str]): names : Union[List[str], str] List of names to combine to form a combined name or the name itself """ - if isinstance(names, str): - names = [names] - return _backend.PrefixGeneratedName(names) + return _backend.PrefixGeneratedName(_preprocess_names(names)) def sanitise_name(original_name: str): diff --git a/src/relay/backend/name_transforms.cc b/src/relay/backend/name_transforms.cc index f09e6a6346a5..3107b2068e96 100644 --- a/src/relay/backend/name_transforms.cc +++ b/src/relay/backend/name_transforms.cc @@ -29,7 +29,8 @@ namespace relay { namespace backend { std::string ToCFunctionStyle(const std::string& original_name) { - ICHECK(!original_name.empty()); + ICHECK(!original_name.empty()) << "Function name is empty"; + ICHECK_EQ(original_name.find("TVM"), 0) << "Function not TVM prefixed"; int tvm_prefix_length = 3; std::string function_name("TVM"); @@ -51,7 +52,8 @@ std::string ToCFunctionStyle(const std::string& original_name) { } std::string ToCVariableStyle(const std::string& original_name) { - ICHECK(!original_name.empty()); + ICHECK(!original_name.empty()) << "Variable name is empty"; + ICHECK_EQ(original_name.find("TVM"), 0) << "Variable not TVM prefixed"; std::string variable_name; variable_name.resize(original_name.size()); @@ -62,10 +64,10 @@ std::string ToCVariableStyle(const std::string& original_name) { std::string CombineNames(const Array& names) { std::stringstream combine_stream; - ICHECK(!names.empty()); + ICHECK(!names.empty()) << "Name segments empty"; for (const String& name : names) { - ICHECK(!name.empty()); + ICHECK(!name.empty()) << "Name segment is empty"; combine_stream << name << "_"; } @@ -75,7 +77,7 @@ std::string CombineNames(const Array& names) { } std::string SanitiseName(const std::string& name) { - ICHECK(!name.empty()); + ICHECK(!name.empty()) << "Name is empty"; auto multipleSeparators = [](char before, char after) { return before == '_' && before == after; diff --git a/tests/cpp/name_transforms_test.cc b/tests/cpp/name_transforms_test.cc index bcad3f9db091..9a5a46950855 100644 --- a/tests/cpp/name_transforms_test.cc +++ b/tests/cpp/name_transforms_test.cc @@ -30,6 +30,7 @@ TEST(NameTransforms, ToCFunctionStyle) { ASSERT_EQ(ToCFunctionStyle("TVM_woof"), "TVMWoof"); ASSERT_EQ(ToCFunctionStyle("TVM_woof_woof"), "TVMWoofWoof"); ASSERT_EQ(ToCFunctionStyle("TVMGen_woof_woof"), "TVMGenWoofWoof"); + EXPECT_THROW(ToCVariableStyle("Cake_Bakery"), InternalError); // Incorrect prefix EXPECT_THROW(ToCFunctionStyle(""), InternalError); } @@ -37,6 +38,7 @@ TEST(NameTransforms, ToCVariableStyle) { ASSERT_EQ(ToCVariableStyle("TVM_Woof"), "tvm_woof"); ASSERT_EQ(ToCVariableStyle("TVM_woof"), "tvm_woof"); ASSERT_EQ(ToCVariableStyle("TVM_woof_Woof"), "tvm_woof_woof"); + EXPECT_THROW(ToCVariableStyle("Cake_Bakery"), InternalError); // Incorrect prefix EXPECT_THROW(ToCVariableStyle(""), InternalError); } diff --git a/tests/python/relay/test_name_transforms.py b/tests/python/relay/test_name_transforms.py index 34ea100392f5..10772d540873 100644 --- a/tests/python/relay/test_name_transforms.py +++ b/tests/python/relay/test_name_transforms.py @@ -32,7 +32,10 @@ def test_to_c_function_style(): assert to_c_function_style("TVM_woof_woof") == "TVMWoofWoof" assert to_c_function_style("TVMGen_woof_woof") == "TVMGenWoofWoof" - with pytest.raises(TVMError): + # Incorrect prefix + with pytest.raises(TVMError, match="Function not TVM prefixed"): + to_c_function_style("Cake_Bakery") + with pytest.raises(TVMError, match="Function name is empty"): to_c_function_style("") @@ -41,7 +44,10 @@ def test_to_c_variable_style(): assert to_c_variable_style("TVM_woof") == "tvm_woof" assert to_c_variable_style("TVM_woof_Woof") == "tvm_woof_woof" - with pytest.raises(TVMError): + # Incorrect prefix + with pytest.raises(TVMError, match="Variable not TVM prefixed"): + to_c_variable_style("Cake_Bakery") + with pytest.raises(TVMError, match="Variable name is empty"): to_c_variable_style("") @@ -51,11 +57,11 @@ def test_prefix_name(): assert prefix_name(["woof"]) == "TVM_woof" assert prefix_name(["woof", "moo"]) == "TVM_woof_moo" - with pytest.raises(TVMError): + with pytest.raises(TVMError, match="Name is empty"): prefix_name("") - with pytest.raises(TVMError): + with pytest.raises(TVMError, match="Name segments empty"): prefix_name([]) - with pytest.raises(TVMError): + with pytest.raises(TVMError, match="Name segment is empty"): prefix_name([""]) @@ -66,11 +72,11 @@ def test_prefix_generated_name(): assert prefix_generated_name(["woof"]) == "TVMGen_woof" assert prefix_generated_name(["woof", "moo"]) == "TVMGen_woof_moo" - with pytest.raises(TVMError): + with pytest.raises(TVMError, match="Name is empty"): prefix_generated_name("") - with pytest.raises(TVMError): + with pytest.raises(TVMError, match="Name segments empty"): prefix_generated_name([]) - with pytest.raises(TVMError): + with pytest.raises(TVMError, match="Name segment is empty"): prefix_generated_name([""]) @@ -81,7 +87,7 @@ def test_sanitise_name(): assert sanitise_name("input++") == "input_" assert sanitise_name("woof:1") == "woof_1" - with pytest.raises(TVMError): + with pytest.raises(TVMError, match="Name is empty"): sanitise_name("") From d4e6c261716fc3091b959c413701afd549f50d75 Mon Sep 17 00:00:00 2001 From: Chris Sidebottom Date: Tue, 28 Sep 2021 11:19:37 +0000 Subject: [PATCH 3/3] Sanitize sanitise to sanitize This patch aims to sanitize uses of sanitise to the form of sanitize to be consistent with the overall codebases use of American English. --- python/tvm/relay/backend/name_transforms.py | 8 ++++---- src/relay/backend/name_transforms.cc | 16 ++++++++-------- src/relay/backend/name_transforms.h | 6 +++--- tests/cpp/name_transforms_test.cc | 14 +++++++------- tests/python/relay/test_name_transforms.py | 16 ++++++++-------- 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/python/tvm/relay/backend/name_transforms.py b/python/tvm/relay/backend/name_transforms.py index 2107485bd1ae..04a7a425bdf1 100644 --- a/python/tvm/relay/backend/name_transforms.py +++ b/python/tvm/relay/backend/name_transforms.py @@ -87,12 +87,12 @@ def prefix_generated_name(names: Union[List[str], str]): return _backend.PrefixGeneratedName(_preprocess_names(names)) -def sanitise_name(original_name: str): - """Sanitise name for output into compiler artifacts +def sanitize_name(original_name: str): + """Sanitize name for output into compiler artifacts Parameters ---------- original_name : str - Original name to sanitise + Original name to sanitize """ - return _backend.SanitiseName(original_name) + return _backend.SanitizeName(original_name) diff --git a/src/relay/backend/name_transforms.cc b/src/relay/backend/name_transforms.cc index 3107b2068e96..a6d10a795cf7 100644 --- a/src/relay/backend/name_transforms.cc +++ b/src/relay/backend/name_transforms.cc @@ -76,28 +76,28 @@ std::string CombineNames(const Array& names) { return combined_name; } -std::string SanitiseName(const std::string& name) { +std::string SanitizeName(const std::string& name) { ICHECK(!name.empty()) << "Name is empty"; auto multipleSeparators = [](char before, char after) { return before == '_' && before == after; }; auto isNotAlnum = [](char c) { return !std::isalnum(c); }; - std::string sanitised_input = name; - std::replace_if(sanitised_input.begin(), sanitised_input.end(), isNotAlnum, '_'); + std::string sanitized_input = name; + std::replace_if(sanitized_input.begin(), sanitized_input.end(), isNotAlnum, '_'); - sanitised_input.erase( - std::unique(sanitised_input.begin(), sanitised_input.end(), multipleSeparators), - sanitised_input.end()); + sanitized_input.erase( + std::unique(sanitized_input.begin(), sanitized_input.end(), multipleSeparators), + sanitized_input.end()); - return sanitised_input; + return sanitized_input; } TVM_REGISTER_GLOBAL("relay.backend.ToCFunctionStyle").set_body_typed(ToCFunctionStyle); TVM_REGISTER_GLOBAL("relay.backend.ToCVariableStyle").set_body_typed(ToCVariableStyle); TVM_REGISTER_GLOBAL("relay.backend.PrefixName").set_body_typed(PrefixName); TVM_REGISTER_GLOBAL("relay.backend.PrefixGeneratedName").set_body_typed(PrefixGeneratedName); -TVM_REGISTER_GLOBAL("relay.backend.SanitiseName").set_body_typed(SanitiseName); +TVM_REGISTER_GLOBAL("relay.backend.SanitizeName").set_body_typed(SanitizeName); } // namespace backend } // namespace relay diff --git a/src/relay/backend/name_transforms.h b/src/relay/backend/name_transforms.h index f94b472f88c8..4c1fd3ae56fc 100644 --- a/src/relay/backend/name_transforms.h +++ b/src/relay/backend/name_transforms.h @@ -92,11 +92,11 @@ inline std::string PrefixGeneratedName(const Array& names) { } /*! - * \brief Sanitise name for output into compiler artifacts + * \brief Sanitize name for output into compiler artifacts * \param name Original name - * \return Sanitised name + * \return Sanitized name */ -std::string SanitiseName(const std::string& name); +std::string SanitizeName(const std::string& name); } // namespace backend } // namespace relay diff --git a/tests/cpp/name_transforms_test.cc b/tests/cpp/name_transforms_test.cc index 9a5a46950855..9fc52e09dea8 100644 --- a/tests/cpp/name_transforms_test.cc +++ b/tests/cpp/name_transforms_test.cc @@ -70,13 +70,13 @@ TEST(NameTransforms, CombineNames) { EXPECT_THROW(CombineNames({"", "Woof"}), InternalError); } -TEST(NameTransforms, SanitiseName) { - ASSERT_EQ(SanitiseName("+_+ "), "_"); - ASSERT_EQ(SanitiseName("input+"), "input_"); - ASSERT_EQ(SanitiseName("input-"), "input_"); - ASSERT_EQ(SanitiseName("input++"), "input_"); - ASSERT_EQ(SanitiseName("woof:1"), "woof_1"); - EXPECT_THROW(SanitiseName(""), InternalError); +TEST(NameTransforms, SanitizeName) { + ASSERT_EQ(SanitizeName("+_+ "), "_"); + ASSERT_EQ(SanitizeName("input+"), "input_"); + ASSERT_EQ(SanitizeName("input-"), "input_"); + ASSERT_EQ(SanitizeName("input++"), "input_"); + ASSERT_EQ(SanitizeName("woof:1"), "woof_1"); + EXPECT_THROW(SanitizeName(""), InternalError); } TEST(NameTransforms, CombinedLogic) { diff --git a/tests/python/relay/test_name_transforms.py b/tests/python/relay/test_name_transforms.py index 10772d540873..c4a7d6c4477c 100644 --- a/tests/python/relay/test_name_transforms.py +++ b/tests/python/relay/test_name_transforms.py @@ -21,7 +21,7 @@ to_c_variable_style, prefix_name, prefix_generated_name, - sanitise_name, + sanitize_name, ) import pytest @@ -80,15 +80,15 @@ def test_prefix_generated_name(): prefix_generated_name([""]) -def test_sanitise_name(): - assert sanitise_name("+_+ ") == "_" - assert sanitise_name("input+") == "input_" - assert sanitise_name("input-") == "input_" - assert sanitise_name("input++") == "input_" - assert sanitise_name("woof:1") == "woof_1" +def test_sanitize_name(): + assert sanitize_name("+_+ ") == "_" + assert sanitize_name("input+") == "input_" + assert sanitize_name("input-") == "input_" + assert sanitize_name("input++") == "input_" + assert sanitize_name("woof:1") == "woof_1" with pytest.raises(TVMError, match="Name is empty"): - sanitise_name("") + sanitize_name("") def test_combined_logic():