search

wlav / cppyy

Other
407 stars 42 forks source link

[Question] Problems while testing with ns-3 #31

Closed Gabrielcarvfer closed 2 years ago

Gabrielcarvfer commented 2 years ago

Sorry, typed enter without writing the text...

Hi, I've been testing the runtime bindings with the ns-3 simulator as we are thinking of dropping pybindgen and would be good to have an alternative.

While setting up cppyy runtime bindings was incredibly easy (90 lines), I'm having some issues.

First is due to a custom operator being overwritten/ignored. It is quite hacky, so I don't mind using a workaround, but wanted to inform you.

ipv4Address = interfaces.GetAddress(1)
address = ns.addressFromIpv4Address(ipv4Address)
# In the entire codebase, operator Address converts Ipv4Address to Address automatically
echoClient = ns.applications.UdpEchoClientHelper(address, 9)

Second issue object lifetime management. I have no idea of the internals, but things like the following fail with a segmentation violation:

networkNode = ns.Node()
nodeContainer = ns.NodeContainer()
nodeContainer.Add(networkNode)

while instantiating the Node in the c++ side works correctly:

nodeContainer = ns.NodeContainer(1)
nodeContainer2 = ns.NodeContainer(nodeContainer.Get(0))

Same issue here and with CommandLine here. Not sure if that is a problem on our side, or just template shenanigans.

Third issue: some methods seem to map to the wrong classes. I don't quite understand why either. With this example:

mobility = ns.mobility.ConstantPositionMobilityModel()
mobility.SetPosition(ns.core.Vector(xi*DISTANCE, yi*DISTANCE, 0))
node.AggregateObject(mobility)

I get 

msg="Object::AggregateObject(): Multiple aggregation of objects of type ns3::Object on objects of type ns3::Object", +0.000000000s -1 file=/mnt/dev/tools/source/ns-3-dev/src/core/model/object.cc, line=279
terminate called without an active exception

The typeId of the aggregated object isn't ns3::Object, but ns3::ConstantPositionMobilityModel.

Fourth issue, which I believe has something to do with the third. In example, I get the following:

remoteAddr = appSink.GetObject(ns.Ipv4.GetTypeId()).GetAddress(1,0).GetLocal()
TypeError: Template method resolution failed:
  Failed to instantiate "GetObject(ns3::TypeId&)"
  Failed to instantiate "GetObject(ns3::TypeId*)"
  Failed to instantiate "GetObject(ns3::TypeId)"

GetObject searches for aggregated objects based on the typeId.

If you have the free time and want to give it a try, cloning this branch, then configuring the project with ./ns3 configure --enable-examples && ./ns3 build, then you should be able to run ./ns3 run example.py --no-build to run a python example.

I'm not really sure of what we can do to make it work.

wlav commented 2 years ago

For the first, yes, the code only looks for copy constructors for implicit conversion, not operators. That can be fixed, but overall, implicit conversion being so unnatural in Python, it's clunky and slow to begin with and that workaround is highly likely to be more efficient.

The second: I'd have to look at the code ... does Add() take ownership on the C++ side?

The third: is ns3::Object a base class of ns3::ConstantPositionMobilityModel and does AggregateObject have overloads for both by reference or pointer? If yes: there's some code to prefer the most derived class, but it's not an exact rule. Upstream has an improvement that I could pull forward.

The fourth can not work: there is no way to derive the return type from the argument, so the instantiation will always fail. How is that code used in C++?

Gabrielcarvfer commented 2 years ago

Being slow isn't that bad in this case, since it would only be used during simulation setup and maybe a few callbacks.

For the second case: I think both cases have shared ownership. Push_back copies the incoming pointer and increments the refcount. 

void NodeContainer::Add (Ptr<Node> node)
{
  m_nodes.push_back (node);
}

The third case: ns3::Object is the base class of ns3::ConstantPositionMobilityModel, but Object::AggregateObject only accepts Ptr pointers. Not completely sure if there is an implicit conversion as the reference can be used to construct a Ptr of the same type.

The fourth: the code for it looks like this.

template <typename T>
Ptr<T>
Object::GetObject (TypeId tid) const
{
  Ptr<Object> found = DoGetObject (tid);
  if (found != 0)
    {
      return Ptr<T> (static_cast<T *> (PeekPointer (found)));
    }
  return 0;
}

I haven't found a single use of it other than in Python examples.

What I found were uses to its overload that always return an Object pointer.

template
<>
inline Ptr<Object>
Object::GetObject (TypeId tid) const
{
  if (tid == Object::GetTypeId ())
    {
      return Ptr<Object> (const_cast<Object *> (this));
    }
 return DoGetObject (tid);
}

For most use cases, we usually use Ptr<type> aggregatedObjectPtr = parentObject->GetObject<type>().

Maybe I just need to write a wrapper taking in __cpp_name__ that calls the underlying c++ code? I think it should work, but I'm not sure how exactly I could patch every single GetObject to keep the same syntax. Adding a ns.GetObject(parentObject, aggregatedObjectType) could work too. I am going to test these.

Thank you for responding. Much appreciated. Sorry if I said something dumb.

Update: it surprisingly does work, but still crashing due to the lifetime issues. Still investigating.

    def GetObject(parentObject, aggregatedObject):
        # Objects have __cpp_name__ attributes, so parentObject
        # should not have it while aggregatedObject can
        if hasattr(parentObject, "__cpp_name__"):
            raise Exception("Class was passed instead of an instance in parentObject")

        aggregatedIsClass = hasattr(aggregatedObject, "__cpp_name__")
        aggregatedIsString = type(aggregatedObject) == str
        aggregatedIsInstance = not aggregatedIsClass and not aggregatedIsString

        if aggregatedIsClass:
            aggregatedType = aggregatedObject.__cpp_name__
        if aggregatedIsInstance:
            aggregatedType = aggregatedObject.__class__.__cpp_name__
        if aggregatedIsString:
            aggregatedType = aggregatedObject

        cppyy.cppdef(
            """using namespace ns3; Ptr<%s> getAggregatedObject(Ptr<Object> parentPtr)
               { 
                    return parentPtr->GetObject<%s>();
               }
            """ % (aggregatedType, aggregatedType)
        )
        return cppyy.gbl.getAggregatedObject(parentObject)
wlav commented 2 years ago

It didn't occur to me that Ptr<T> was a smart ptr. There is a pythonization interface to declare classes as smart ptrs. Never got around to documenting it. Let me found out what the status is, and work through an example in the documentation, as it may solve the life time problems.

wlav commented 2 years ago

The interface to register smart pointers isn't actually accessible through the pythonizations, but can be used as:

import libcppyy
libcppyy.AddSmartPtrType('Ptr')

although the only benefit at this point is that you can pass a Ptr<Node> through a Node*.

It does not solve the memory issue by setting the argument's __python_owns__ to False.

What I don't understand, is that if Ptr is NOT declared smart, there should be no smart pointer conversion (classes of Ptr<Base> and Ptr<Derived> would be seen as unrelated), whereas the case here seems to be the opposite. That is, only if declared smart, should derived smart be able to pass through base smart.

Anyway, looks like this interface will need a bit of work to revive the original intent ...

Gabrielcarvfer commented 2 years ago

@wlav thanks for your help and dwork. We already switched to cppyy and it has been working just fine. I'm going to close this issue. :)

  • © Githubissues.
  • Githubissues is a development platform for aggregating issues.