From ce4d9584d979a8796d3abd991afd5957c853f7df Mon Sep 17 00:00:00 2001 From: Olivier BICHLER <olivier.bichler@cea.fr> Date: Sun, 22 Sep 2024 19:16:12 +0200 Subject: [PATCH] Working concept --- .../unit_tests/test_operator_binding.py | 2 +- include/aidge/operator/GenericOperator.hpp | 2 +- include/aidge/utils/Attributes.hpp | 2 - include/aidge/utils/DynamicAttributes.hpp | 335 +++++++----------- include/aidge/utils/StaticAttributes.hpp | 12 - python_binding/utils/pybind_Attributes.cpp | 4 +- src/utils/DynamicAttributes.cpp | 8 +- 7 files changed, 139 insertions(+), 226 deletions(-) diff --git a/aidge_core/unit_tests/test_operator_binding.py b/aidge_core/unit_tests/test_operator_binding.py index 8d6f2686d..21b62e92b 100644 --- a/aidge_core/unit_tests/test_operator_binding.py +++ b/aidge_core/unit_tests/test_operator_binding.py @@ -101,7 +101,7 @@ class test_operator_binding(unittest.TestCase): self.assertEqual(aidge_core.test_DynamicAttributes_binding_check(attrs), 23.89) op = aidge_core.GenericOperatorOp("any_type", 1,0,1) - with self.assertRaises(RuntimeError): + with self.assertRaises(IndexError): op.attr.something op.attr.something = aidge_core.DynamicAttributes() diff --git a/include/aidge/operator/GenericOperator.hpp b/include/aidge/operator/GenericOperator.hpp index 2812da066..89b2c06a5 100644 --- a/include/aidge/operator/GenericOperator.hpp +++ b/include/aidge/operator/GenericOperator.hpp @@ -64,7 +64,7 @@ public: inline T& getAttr(const std::string& name) { return mAttributes -> template getAttr<T>(name); } template <class T> - inline const T& getAttr(const std::string& name) const + inline T getAttr(const std::string& name) const { return mAttributes -> template getAttr<T>(name); } ///\brief Add a new Attribute, identified by its name. If it already exists, asserts. diff --git a/include/aidge/utils/Attributes.hpp b/include/aidge/utils/Attributes.hpp index cf71ed0b5..fd29bf4ce 100644 --- a/include/aidge/utils/Attributes.hpp +++ b/include/aidge/utils/Attributes.hpp @@ -69,8 +69,6 @@ public: virtual std::map<std::string, future_std::any> getAttrs() const = 0; #ifdef PYBIND - virtual bool hasAttrPy(const std::string& name) const = 0; - /* Bindable get function, does not recquire any templating. * This is thanks to py::object which allow the function to * be agnostic from its return type. diff --git a/include/aidge/utils/DynamicAttributes.hpp b/include/aidge/utils/DynamicAttributes.hpp index 04ed58f7e..659f61519 100644 --- a/include/aidge/utils/DynamicAttributes.hpp +++ b/include/aidge/utils/DynamicAttributes.hpp @@ -39,8 +39,12 @@ namespace Aidge { ///\todo managing complex types or excluding non-trivial, non-aggregate types class DynamicAttributes : public Attributes { public: - DynamicAttributes() = default; - DynamicAttributes(const std::map<std::string, future_std::any>& attrs): mAttrs(attrs) {} + DynamicAttributes() { + mAnyUtils.emplace(typeid(DynamicAttributes), std::unique_ptr<AnyUtils<DynamicAttributes>>(new AnyUtils<DynamicAttributes>())); + } + DynamicAttributes(const std::map<std::string, future_std::any>& attrs): mAttrs(attrs) { + mAnyUtils.emplace(typeid(DynamicAttributes), std::unique_ptr<AnyUtils<DynamicAttributes>>(new AnyUtils<DynamicAttributes>())); + } /** * \brief Returning an Attribute identified by its name @@ -50,39 +54,23 @@ public: * exist * \note at() throws if the Attribute does not exist, using find to test for Attribute existance */ - template<class T> const T& getAttr(const std::string& name) const + template<class T> T getAttr(const std::string& name) const { - mAnyCompare.emplace(std::make_pair<std::type_index, bool(*)(const future_std::any&, const future_std::any&)>(typeid(T), - [](const future_std::any& lhs, const future_std::any& rhs) { -#ifdef PYBIND - if (lhs.type() == typeid(py::object)) { - return (future_std::any_cast<py::object>(lhs).cast<T>() < future_std::any_cast<T>(rhs)); - } - else if (rhs.type() == typeid(py::object)) { - return (future_std::any_cast<T>(lhs) < future_std::any_cast<py::object>(rhs).cast<T>()); - } - else -#endif - { - return (future_std::any_cast<T>(lhs) < future_std::any_cast<T>(rhs)); - } - })); - const auto dot = name.find('.'); if (dot == name.npos) { + mAnyUtils.emplace(typeid(T), std::unique_ptr<AnyUtils<T>>(new AnyUtils<T>())); + + const auto& attr = mAttrs.at(name); #ifdef PYBIND - // If attribute does not exist in C++, it might have been created or modified in Python - auto it = mAttrs.find(name); - if (it == mAttrs.end()) { - auto itPy = mAttrsPy.find(name); - if (itPy != mAttrsPy.end()) { - // Insert the attribute back in C++ - mAttrs.emplace(std::make_pair(name, future_std::any(itPy->second.cast<T>()))); - } + if (attr.type() == typeid(py::object)) { + // Note: because of cast<T>(), this function cannot return a const reference! + return future_std::any_cast<const py::object&>(attr).cast<T>(); } + else #endif - - return future_std::any_cast<const T&>(mAttrs.at(name)); + { + return future_std::any_cast<const T&>(attr); + } } else { const auto ns = name.substr(0, dot); @@ -92,9 +80,21 @@ public: } template<class T> T& getAttr(const std::string& name) { - // Scott Meyers' solution to avoid code duplication - return const_cast<T&>( - static_cast<const DynamicAttributes&>(*this).getAttr<T>(name)); + const auto dot = name.find('.'); + if (dot == name.npos) { + mAnyUtils.emplace(typeid(T), std::unique_ptr<AnyUtils<T>>(new AnyUtils<T>())); + + auto& attr = mAttrs.at(name); +#ifdef PYBIND + AIDGE_ASSERT(attr.type() != typeid(py::object), "getAttr(): cannot return a reference to a Python-defined attribute."); +#endif + return future_std::any_cast<T&>(attr); + } + else { + const auto ns = name.substr(0, dot); + const auto nsName = name.substr(dot + 1); + return future_std::any_cast<DynamicAttributes&>(mAttrs.at(ns)).getAttr<T>(nsName); + } } ///\brief Add a new Attribute, identified by its name. If it already exists, asserts. @@ -103,35 +103,12 @@ public: ///\param value Attribute value template<class T> void addAttr(const std::string& name, const T& value) { - mAnyCompare.emplace(std::make_pair<std::type_index, bool(*)(const future_std::any&, const future_std::any&)>(typeid(T), - [](const future_std::any& lhs, const future_std::any& rhs) { -#ifdef PYBIND - if (lhs.type() == typeid(py::object)) { - return (future_std::any_cast<py::object>(lhs).cast<T>() < future_std::any_cast<T>(rhs)); - } - else if (rhs.type() == typeid(py::object)) { - return (future_std::any_cast<T>(lhs) < future_std::any_cast<py::object>(rhs).cast<T>()); - } - else -#endif - { - return (future_std::any_cast<T>(lhs) < future_std::any_cast<T>(rhs)); - } - })); - const auto dot = name.find('.'); if (dot == name.npos) { + mAnyUtils.emplace(typeid(T), std::unique_ptr<AnyUtils<T>>(new AnyUtils<T>())); + const auto& res = mAttrs.emplace(std::make_pair(name, future_std::any(value))); AIDGE_ASSERT(res.second, "addAttr(): attribute \"{}\" already exists. Use setAttr() if this is expected.", name); - -#ifdef PYBIND - // We cannot handle Python object if the Python interpreter is not running - if (Py_IsInitialized()) { - // Keep a copy of the attribute in py::object that is updated everytime - const auto& resPy = mAttrsPy.emplace(std::make_pair(name, py::cast(value))); - AIDGE_ASSERT(resPy.second, "addAttr(): attribute \"{}\" already exists (added in Python). Use setAttr() if this is expected.", name); - } -#endif } else { const auto ns = name.substr(0, dot); @@ -147,37 +124,13 @@ public: ///\param value Attribute value template<class T> void setAttr(const std::string& name, const T& value) { - mAnyCompare.emplace(std::make_pair<std::type_index, bool(*)(const future_std::any&, const future_std::any&)>(typeid(T), - [](const future_std::any& lhs, const future_std::any& rhs) { -#ifdef PYBIND - if (lhs.type() == typeid(py::object)) { - return (future_std::any_cast<py::object>(lhs).cast<T>() < future_std::any_cast<T>(rhs)); - } - else if (rhs.type() == typeid(py::object)) { - return (future_std::any_cast<T>(lhs) < future_std::any_cast<py::object>(rhs).cast<T>()); - } - else -#endif - { - return (future_std::any_cast<T>(lhs) < future_std::any_cast<T>(rhs)); - } - })); - const auto dot = name.find('.'); if (dot == name.npos) { + mAnyUtils.emplace(typeid(T), std::unique_ptr<AnyUtils<T>>(new AnyUtils<T>())); + auto res = mAttrs.emplace(std::make_pair(name, future_std::any(value))); if (!res.second) res.first->second = future_std::any(value); - -#ifdef PYBIND - // We cannot handle Python object if the Python interpreter is not running - if (Py_IsInitialized()) { - // Keep a copy of the attribute in py::object that is updated everytime - auto resPy = mAttrsPy.emplace(std::make_pair(name, py::cast(value))); - if (!resPy.second) - resPy.first->second = std::move(py::cast(value)); - } -#endif } else { const auto ns = name.substr(0, dot); @@ -191,9 +144,6 @@ public: const auto dot = name.find('.'); if (dot == name.npos) { mAttrs.erase(name); -#ifdef PYBIND - mAttrsPy.erase(name); -#endif } else { const auto ns = name.substr(0, dot); @@ -205,41 +155,12 @@ public: #ifdef PYBIND void addAttrPy(const std::string& name, py::object&& value) { - const auto dot = name.find('.'); - if (dot == name.npos) { - auto it = mAttrs.find(name); - AIDGE_ASSERT(it == mAttrs.end(), "add_attr(): attribute \"{}\" already exists (added in C++). Use set_attr() if this is expected.", name); - - const auto& res = mAttrsPy.emplace(std::make_pair(name, value)); - AIDGE_ASSERT(res.second, "add_attr(): attribute \"{}\" already exists. Use set_attr() if this is expected.", name); - } - else { - const auto ns = name.substr(0, dot); - const auto nsName = name.substr(dot + 1); - const auto& res = mAttrs.emplace(std::make_pair(ns, DynamicAttributes())); - - future_std::any_cast<DynamicAttributes&>(res.first->second).addAttrPy(nsName, std::move(value)); - } + addAttr(name, std::move(value)); } void setAttrPy(const std::string& name, py::object&& value) override final { - const auto dot = name.find('.'); - if (dot == name.npos) { - auto resPy = mAttrsPy.emplace(std::make_pair(name, value)); - if (!resPy.second) - resPy.first->second = std::move(value); - - // Force getAttr() to take attribute value from mAttrsPy and update mAttrs - mAttrs.erase(name); - } - else { - const auto ns = name.substr(0, dot); - const auto nsName = name.substr(dot + 1); - const auto& res = mAttrs.emplace(std::make_pair(ns, DynamicAttributes())); - - future_std::any_cast<DynamicAttributes&>(res.first->second).setAttrPy(nsName, std::move(value)); - } + setAttr(name, std::move(value)); } py::dict dict() const override { @@ -248,9 +169,9 @@ public: if (elt.second.type() == typeid(DynamicAttributes)) { attributes[elt.first.c_str()] = future_std::any_cast<const DynamicAttributes&>(elt.second).dict(); } - } - for (const auto& elt : mAttrsPy) { - attributes[elt.first.c_str()] = elt.second; + else { + attributes[elt.first.c_str()] = mAnyUtils.at(elt.second.type())->cast(elt.second); + } } return attributes; } @@ -273,12 +194,7 @@ public: bool hasAttr(const std::string& name) const override final { const auto dot = name.find('.'); if (dot == name.npos) { -#ifdef PYBIND - return (mAttrs.find(name) != mAttrs.cend() || mAttrsPy.find(name) != mAttrsPy.cend()); - -#else return (mAttrs.find(name) != mAttrs.cend()); -#endif } else { const auto ns = name.substr(0, dot); @@ -293,45 +209,22 @@ public: } } -#ifdef PYBIND - bool hasAttrPy(const std::string& name) const override final { - const auto dot = name.find('.'); - if (dot == name.npos) { - // Attributes might have been created in Python, the second condition is necessary. - return (mAttrs.find(name) != mAttrs.cend() || mAttrsPy.find(name) != mAttrsPy.cend()); - } - else { - const auto ns = name.substr(0, dot); - const auto it = mAttrs.find(ns); - if (it != mAttrs.cend()) { - const auto nsName = name.substr(dot + 1); - return future_std::any_cast<const DynamicAttributes&>(it->second).hasAttrPy(nsName); - } - else { - return false; - } - } - } -#endif - std::string getAttrType(const std::string& name) const override final { // In order to remain consistent between C++ and Python, with or without PyBind, the name of the type is: // - C-style for C++ created attributes // - Python-style for Python created attributes const auto dot = name.find('.'); if (dot == name.npos) { + const auto& attr = mAttrs.at(name); #ifdef PYBIND - // If attribute does not exist in C++, it might have been created in Python - auto it = mAttrs.find(name); - if (it == mAttrs.end()) { - auto itPy = mAttrsPy.find(name); - if (itPy != mAttrsPy.end()) { - return std::string(Py_TYPE(itPy->second.ptr())->tp_name); - } + if (attr.type() == typeid(py::object)) { + return std::string(Py_TYPE(future_std::any_cast<const py::object&>(attr).ptr())->tp_name); } + else #endif - - return mAttrs.at(name).type().name(); + { + return attr.type().name(); + } } else { const auto ns = name.substr(0, dot); @@ -344,11 +237,6 @@ public: std::set<std::string> attrsName; for(auto const& it: mAttrs) attrsName.insert(it.first); -#ifdef PYBIND - // Attributes might have been created in Python - for(auto const& it: mAttrsPy) - attrsName.insert(it.first); -#endif return attrsName; } @@ -356,21 +244,13 @@ public: /** * @detail See https://github.com/pybind/pybind11/issues/1590 as to why a * generic type caster for std::any is not feasable. - * The strategy here is to keep a copy of each attribute in py::object that is updated everytime. + * The strategy here is to store a cast() function for each attribute type ever used. */ inline py::object getAttrPy(const std::string& name) const override final { const auto dot = name.find('.'); if (dot == name.npos) { - auto itPy = mAttrsPy.find(name); - if (itPy == mAttrsPy.end()) { - // Attribute may be a namespace - auto it = mAttrs.find(name); - AIDGE_ASSERT(it != mAttrs.end() && it->second.type() == typeid(DynamicAttributes), "get_attr(): attribute \"{}\" not found", name); - return py::cast(future_std::any_cast<const DynamicAttributes&>(it->second)); - } - else { - return itPy->second; - } + const auto& attr = mAttrs.at(name); + return mAnyUtils.at(attr.type())->cast(attr); } else { const auto ns = name.substr(0, dot); @@ -384,24 +264,6 @@ public: { const auto dot = name.find('.'); if (dot == name.npos) { -#ifdef PYBIND - // If attribute does not exist in C++, it might have been created or modified in Python - auto it = mAttrs.find(name); - if (it == mAttrs.end()) { - auto itPy = mAttrsPy.find(name); - if (itPy != mAttrsPy.end()) { - // Attribute exists in Python, but its type is not known - // Return a std::any of py::object, which will be comparable - mAnyCompare.emplace(std::make_pair<std::type_index, bool(*)(const future_std::any&, const future_std::any&)>(typeid(py::object), - [](const future_std::any& lhs, const future_std::any& rhs) { - return (future_std::any_cast<py::object>(lhs) < future_std::any_cast<py::object>(rhs)); - })); - - return future_std::any(itPy->second); - } - } -#endif - return mAttrs.at(name); } else { @@ -418,33 +280,98 @@ public: virtual ~DynamicAttributes() {} friend bool operator<(const DynamicAttributes& lhs, const DynamicAttributes& rhs); + friend struct std::hash<DynamicAttributes>; private: -#ifdef PYBIND - // Stores C++ attributes (copy) and Python-only attributes - // Code should be compiled with -fvisibility=hidden - // See https://pybind11.readthedocs.io/en/stable/faq.html: - // “‘SomeClass’ declared with greater visibility than the type of its - // field ‘SomeClass::member’ [-Wattributes]†- // This map will only be populated if Python interpreter is running - std::map<std::string, py::object> mAttrsPy; - // Stores C++ attributes only - // mutable because it may be updated in getAttr() from Python - mutable std::map<std::string, future_std::any> mAttrs; -#else std::map<std::string, future_std::any> mAttrs; -#endif public: - // Stores the comparison function for each attribute type ever used - static std::map<std::type_index, bool(*)(const future_std::any&, const future_std::any&)> mAnyCompare; + struct AnyUtils_ { +#ifdef PYBIND + virtual py::object cast(const future_std::any& attr) const = 0; +#endif + virtual bool compare(const future_std::any&, const future_std::any&) const = 0; + virtual size_t hash(const future_std::any&) const = 0; + virtual ~AnyUtils_() = default; + }; + + template <class T> + struct AnyUtils : public AnyUtils_ { +#ifdef PYBIND + py::object cast(const future_std::any& attr) const override final { + return py::cast(future_std::any_cast<const T&>(attr)); + } +#endif + + bool compare(const future_std::any& lhs, const future_std::any& rhs) const override final { +#ifdef PYBIND + if (lhs.type() == typeid(py::object) && rhs.type() != typeid(py::object)) { + return (future_std::any_cast<py::object>(lhs).cast<T>() < future_std::any_cast<T>(rhs)); + } + else if (lhs.type() != typeid(py::object) && rhs.type() == typeid(py::object)) { + return (future_std::any_cast<T>(lhs) < future_std::any_cast<py::object>(rhs).cast<T>()); + } + else +#endif + { + return (future_std::any_cast<T>(lhs) < future_std::any_cast<T>(rhs)); + } + } + + size_t hash(const future_std::any& attr) const override final { + return std::hash<T>()(future_std::any_cast<T>(attr)); + } + }; + + // Stores typed utils functions for each attribute type ever used + static std::map<std::type_index, std::unique_ptr<AnyUtils_>> mAnyUtils; }; +#ifdef PYBIND +template <> +struct DynamicAttributes::AnyUtils<py::object> : public DynamicAttributes::AnyUtils_ { + py::object cast(const future_std::any& attr) const override { + return future_std::any_cast<const py::object&>(attr); + } + + bool compare(const future_std::any& lhs, const future_std::any& rhs) const override { + return (future_std::any_cast<py::object>(lhs) < future_std::any_cast<py::object>(rhs)); + } + + size_t hash(const future_std::any& attr) const override final { + // Here we are mixing Python and C++ hashes... if both are + // well implemented, this should not increase the collision + // probability for the same number of stored hashes. + return py::hash(future_std::any_cast<py::object>(attr)); + } +}; +#endif + inline bool operator<(const DynamicAttributes& lhs, const DynamicAttributes& rhs) { return (lhs.mAttrs < rhs.mAttrs); } } +namespace std { + // Make DynamicAttributes hashable so that is can be stored in hash-based containers. + // This is particularly useful in Python since set() and dict() are hash-based. + template <> + struct hash<Aidge::DynamicAttributes> { + size_t operator()(const Aidge::DynamicAttributes& attrs) const { + std::size_t seed = 0; + for (const auto& pair : attrs.mAttrs) { + const std::size_t key_hash = std::hash<std::string>()(pair.first); + // Combine the hashes (boost-like hash combining, see boost::hash_combine()) + seed ^= key_hash + 0x9e3779b9 + (seed << 6) + (seed >> 2); + const std::size_t value_hash = Aidge::DynamicAttributes::mAnyUtils.at(pair.second.type())->hash(pair.second); + // Combine the hashes (boost-like hash combining, see boost::hash_combine()) + seed ^= value_hash + 0x9e3779b9 + (seed << 6) + (seed >> 2); + } + return seed; + } + }; +} + namespace future_std { bool operator<(const future_std::any& lhs, const future_std::any& rhs); } diff --git a/include/aidge/utils/StaticAttributes.hpp b/include/aidge/utils/StaticAttributes.hpp index 414381891..636863e29 100644 --- a/include/aidge/utils/StaticAttributes.hpp +++ b/include/aidge/utils/StaticAttributes.hpp @@ -199,18 +199,6 @@ public: return false; } -#ifdef PYBIND - bool hasAttrPy(const std::string& name) const override final { - for (std::size_t i = 0; i < size(EnumStrings<ATTRS_ENUM>::data); ++i) { - if (name == EnumStrings<ATTRS_ENUM>::data[i]) { - return true; - } - } - - return false; - } -#endif - // Runtime type access with name std::string getAttrType(const std::string& name) const override final { for (std::size_t i = 0; i < size(EnumStrings<ATTRS_ENUM>::data); ++i) { diff --git a/python_binding/utils/pybind_Attributes.cpp b/python_binding/utils/pybind_Attributes.cpp index bc0ccb3f4..691691a86 100644 --- a/python_binding/utils/pybind_Attributes.cpp +++ b/python_binding/utils/pybind_Attributes.cpp @@ -30,13 +30,13 @@ DynamicAttributes test_DynamicAttributes_binding() { return attrs; } -double test_DynamicAttributes_binding_check(DynamicAttributes& attrs) { +double test_DynamicAttributes_binding_check(const DynamicAttributes& attrs) { return attrs.getAttr<double>("d"); } void init_Attributes(py::module& m){ py::class_<Attributes, std::shared_ptr<Attributes>>(m, "Attributes") - .def("has_attr", &Attributes::hasAttrPy, py::arg("name")) + .def("has_attr", &Attributes::hasAttr, py::arg("name")) .def("get_attr", &Attributes::getAttrPy, py::arg("name")) .def("__getattr__", &Attributes::getAttrPy, py::arg("name")) .def("set_attr", &Attributes::setAttrPy, py::arg("name"), py::arg("value")) diff --git a/src/utils/DynamicAttributes.cpp b/src/utils/DynamicAttributes.cpp index 909d3bb2f..facf34377 100644 --- a/src/utils/DynamicAttributes.cpp +++ b/src/utils/DynamicAttributes.cpp @@ -11,18 +11,18 @@ #include "aidge/utils/DynamicAttributes.hpp" -std::map<std::type_index, bool(*)(const future_std::any&, const future_std::any&)> Aidge::DynamicAttributes::mAnyCompare; +std::map<std::type_index, std::unique_ptr<Aidge::DynamicAttributes::AnyUtils_>> Aidge::DynamicAttributes::mAnyUtils; bool future_std::operator<(const future_std::any& lhs, const future_std::any& rhs) { if (lhs.type() == rhs.type()) { - return Aidge::DynamicAttributes::mAnyCompare.at(lhs.type())(lhs, rhs); + return Aidge::DynamicAttributes::mAnyUtils.at(lhs.type())->compare(lhs, rhs); } #ifdef PYBIND else if (lhs.type() == typeid(py::object)) { - return Aidge::DynamicAttributes::mAnyCompare.at(rhs.type())(lhs, rhs); + return Aidge::DynamicAttributes::mAnyUtils.at(rhs.type())->compare(lhs, rhs); } else if (rhs.type() == typeid(py::object)) { - return Aidge::DynamicAttributes::mAnyCompare.at(lhs.type())(lhs, rhs); + return Aidge::DynamicAttributes::mAnyUtils.at(lhs.type())->compare(lhs, rhs); } #endif else { -- GitLab