z25 / pyZOCP

Python ZOCP implementation (Z25 Orchestration Control Protocol)
GNU Lesser General Public License v3.0
33 stars 5 forks source link

Capability organisation #42

Open fieldOfView opened 9 years ago

fieldOfView commented 9 years ago

Currently the capabilities of a node is a dict, keyed by the name of each capability. The both the get/set, mod and signals implementations use this name to communicate the capability reference. This has a couple of disadvantages:

Capabilities could instead be stored in a list. The index of a capability in the list could be used to reference the capability in set/get/mod/sig/sub/unsub messages, as it is unique and does not change over the lifetime of a node.

fieldOfView commented 9 years ago

Forgot to mention: Cases where a node needs to iterate over all capabilities to find the capability with a certain name should be minimal. The register_* methods return the index of the capability in the list, so the node knows what index relates to what registered capability.

sphaero commented 9 years ago

I've been working on this @z25. see http://projects.z25.org/projects/plab/wiki/Orchestrator_Protocol#Example-illustrative-capability-tree

But the essence of how I was thinking of the capability tree: self.capabilities:

# Node name is handled by Pyre since ZRE 2
{'_position: { 'access'   : 'r',                 # this a read only value, no emitter, no receiving
               'typeHint' : 'vec3f',
               'signature': 'fff',
               'value'    : 1                    # Index of the value in the values list (LUT)
               'wgs84'    : [52.083, 5.133] },   # GPS coords
 '_objects': {'3DVideoCanvas': {'position'    : {'access'   : 'rwes',            # r=read, w=write, e=signal emitter, s=signal receiver
                                                 'typeHint' : 'vec3f',
                                                 'signature': 'fff',             # three floats
                                                 'value'    : 0,                 # value is a reference! should provide getters/setters
                                                 'signalID' : '0' },             # signal ID if an emitter/receiver
                               {'orientation' : {'access'   : 'rwes',
                                                 'typeHint' : 'euler',
                                                 'signature': 'fff',
                                                 'signalID' : '1',
                                                 'value'    : 2 },               # orientation euler in radians
                               {'stream-sinks': {'GstSink'  : { 'caps': 'application/x-rtp,encoding-name=JPEG,payload=26',
                                                                'sink': 'rtp://192.168.12.13:5200'}
                               }},
              {'LedBar':       {'text'        : {'access'   : 'rws',
                                                 'typeHint' : 'string',
                                                 'value'    : 3 }
              }}}

self.values:

[
 [2.3, 1.0, 1.0],
 [5.0, 5, 0, 2.3],  # x=5m, y=5m, z=2.30m
 [0.0, 3.14, 0.0],
 "Bloemkolen",
]

self.callbacks

[
 [ get, set ],
 [ get, set ],
 [ get, set ],
 [ get, set ]
]

Hope it is self explanatory. Basically values are referred to through a lookup tables. Same counts for getters and setters. Still have to think about concurrent implications though.

fieldOfView commented 9 years ago

But the above still has the capabilities as a dict, instead of a list. It has a "signalid", but I would think that a "capabilityid" would be a better idea in the new model where every capability can be an emitter and/or a receiver. I propose that this "capabilityid" would simply be the index in the capability list.

sphaero commented 9 years ago

I know. I was only changing the capability description to bypass operations on the dicts.

Indeed the capabilities can be a list of dicts. This protects the order.

I don't follow why the capability ids? The capabilities describe much more than values/signals? Are you suggesting to use an id for a capability leaf? This would be possible and perhaps a bit more flexible? It is essentially the same. Either you create an id to to the value and getters/setters or you use the branch/leave id.

On Thu, 18 Dec 2014, Aldo Hoeben wrote:

But the above still has the capabilities as a dict, instead of a list. It has a "signalid", but I would think that a "capabilityid" would be a better idea in the new model where every capability can be an emitter and/or a receiver. I propose that this "capabilityid" would simply be the index in the capability list.

— Reply to this email directly or view it on GitHub.[AAyz0XCG8zNGInGZ3pg42A-4HKOsEzO0ks5nYqzpgaJpZM4DJZuS.gif]

sphaero commented 9 years ago

I've tested dictionary modification and list modification. I wanted to know the performance difference. I created a simple capability tree and modified values on it. The same modification I also did in a list mimicking a lookup table of values:

Results of three runs are: python2:

python3:

In general it is twice as fast.

# zocp cap dict test
import time

# init cap tree
cur_obj = {}
capabilities = { '_position': { 'access': 'r', 'typeHint' : 'vec3f', 'signature': 'fff', 'value': 1, 'wgs84': [52.083, 5.133] }, '_objects': cur_obj }
cur_obj["test1"] = {'value': 10, 'typeHint': 'int', 'access':"rw", 'subscribers': [] }
cur_obj["test2"] = {'value': 20, 'typeHint': 'int', 'access':"rw", 'subscribers': [] }
cur_obj["test3"] = {'value': 30, 'typeHint': 'int', 'access':"rw", 'subscribers': [] }
cur_obj["test4"] = {'value': 40, 'typeHint': 'int', 'access':"rw", 'subscribers': [] }
cur_obj["test5"] = {'value': 50, 'typeHint': 'int', 'access':"rw", 'subscribers': [] }
cur_obj["test6"] = {'value': 60, 'typeHint': 'int', 'access':"rw", 'subscribers': [] }
cur_obj["test7"] = {'value': 70, 'typeHint': 'int', 'access':"rw", 'subscribers': [] }
cur_obj["test8"] = {'value': 80, 'typeHint': 'int', 'access':"rw", 'subscribers': [] }
cur_obj["test9"] = {'value': 90, 'typeHint': 'int', 'access':"rw", 'subscribers': [] }
cur_obj["test10"] = {'value': 100, 'typeHint': 'int', 'access':"rw", 'subscribers': [] }

# now modify values
t = time.time()
for x in range(100000):
    capabilities['_objects']['test1']['value'] = x
    capabilities['_objects']['test2']['value'] = x
    capabilities['_objects']['test3']['value'] = x
    capabilities['_objects']['test4']['value'] = x
    capabilities['_objects']['test5']['value'] = x
    capabilities['_objects']['test6']['value'] = x
    capabilities['_objects']['test7']['value'] = x
    capabilities['_objects']['test8']['value'] = x
    capabilities['_objects']['test9']['value'] = x
    capabilities['_objects']['test10']['value'] = x
print("mod time:\t{0} ms".format((time.time() -  t)*1000))

t = time.time()
values = list(range(10))
for x in range(100000):
    values[0] = x
    values[1] = x
    values[2] = x
    values[3] = x
    values[4] = x
    values[5] = x
    values[6] = x
    values[7] = x
    values[8] = x
    values[9] = x
print("mod time:\t{0} ms".format((time.time() -  t)*1000))
sphaero commented 9 years ago

I've added another test using a dict (hashmap) as a LUT. To make it bit more real I used three characters for the identifier. Actualy I found a small issue in the first test since the measurement included generating the list. The list is thus a bit faster. The dict is bit slower than the list (of course) But it comes with the benefit of having non ordered identifiers

values = {'-0-': 0, '-1-': 1, '-2-': 2, '-3-': 3, '-4-': 4, '-5-': 5, '-6-': 6, '-7-': 8, '-9-': 9}
t = time.time()
for x in range(100000):
    values['-0-'] = x
    values['-1-'] = x
    values['-2-'] = x
    values['-3-'] = x
    values['-4-'] = x
    values['-5-'] = x
    values['-6-'] = x
    values['-7-'] = x
    values['-8-'] = x
    values['-9-'] = x
print("mod time:\t{0} ms".format((time.time() -  t)*1000))
sphaero commented 9 years ago

@fieldOfView : "I propose that this "capabilityid" would simply be the index in the capability list." Ah I get it. What happens when you remove an object? Then we would need to sync the lists. So when removing an object from the list you need to know the index of the object. If we maintain an id in the object entry then you don't need to sync the lists (and methods and values will also never be garbage collected).... I guess we have to do some maintainance afterall

fieldOfView commented 9 years ago

By "object" you mean "capability"? There currently is no way to remove a capability from the tree. Of course a node could unset a capability, but there is no way to communicate this to the other nodes because (partial) trees sent with a MOD message are merged with the current tree.

Do we want to be able to mark a capability as deleted/inactive/permanently disabled? That way, all indices would be maintained.

sphaero commented 9 years ago

ow yes by object I meant an entry in the objects list. Eventually I think we will need to support some way of removing objects again from the list. It's like the hitman approach. Always cleanup after a job. Perhaps then it might be better to enhance MOD with adds, updates and deletes?

sphaero commented 9 years ago

K, I've done some more metric tests. Contrary to my belief using an array or a numpy array is even slower for our purpose. Numpy and arrays are apparently designed for large data.

1 million operations on 10 values in an dict/list/array:

nested dict mod time:   1109.7524166107178 ms
list        mod time:   652.9648303985596 ms
dictionary  mod time:   625.1497268676758 ms
array       mod time:   1618.0355548858643 ms
numpy array mod time:   2644.8681354522705 ms

However same test in C using an array:

modtime: 6.508000 ms

See also: http://stackoverflow.com/a/23331775

sphaero commented 9 years ago

Also nice to see metrics from pypy:

('nested dict\t',)
mod time:   309.462070465 ms
('list\t\t',)
mod time:   15.517950058 ms
('dictionary\t',)
mod time:   124.591112137 ms
('array\t\t',)
mod time:   147.526025772 ms
('numpy array\t',)
sphaero commented 9 years ago

K, made a slight brain error. We can't use arrays for our values as we can't mix value types in arrays. So I was thinking about getters and setters again and it turns out Python does have an approach to that:

class Prop(object):
    def __init__(self, my_prop, *args, **kwargs):
        self.my_prop = my_prop

    def set_prop(self, prop):
        print("SET")
        self._my_prop = prop

    def get_prop(self):
        print("GET")
        return self._my_prop

    my_prop = property(get_prop, set_prop)

a = Prop(10)
print("inited")
print(a.my_prop)
a.my_prop = 20
print("changed val")
print(a.my_prop)

gives:

SET
inited
GET
10
SET
changed val
GET
20
sphaero commented 9 years ago

Interesting discussion: https://github.com/openframeworks/openFrameworks/issues/1391

sphaero commented 9 years ago

I was looking into having a zocp property class which wraps any object. I'm not there yet as I can't copy the class attributes but the structure does work:

class ZOCPParameter(object):
    """
    Wrapper class for values managed through ZOCP

    The instance can be used just as you would use the variable it holds
    """
    _id = 0                 # id counter

    def __init__(self, znode, value, name, min, max, access, type_hint, signature, *args, **kwargs):
        self._value = value
        # wrap all members
        for member in dir(self._value):
            if member != '__class__': #otherwise it fails :(
                setattr(self, member, getattr(value, member))
        # init meta data
        self._znode = znode
        self.name = name
        self.min =  min
        self.max = max
        self.access = access
        self.type_hint = type_hint
        self.signature = signature
        # get ourselves an id
        self.sig_id = ZOCPParameter._id
        ZOCPParameter._id += 1
        # in case we're an emitter overwrite the set method
        if 'e' in self.access:
            self.set = self._set_emit

    def _set_emit(self, value):
        """
        Set and emit value as a signal
        """ 
        self._value = value
        self._znode.emit_signal(self.sig_id, value)

    def get(self):
        return self._value

    def set(self, value):
        self._value = value

    value = property(get, set)
sphaero commented 9 years ago

Some refs:

sphaero commented 9 years ago

K another thought iteration. Here's a pseudo class of a ZOCP parameter (or what we call it) Instances of this class can be referenced to by its ID. The instance manages its own flow. It can emit signals, manage subscriptions etc... ZOCP only collects the instances in a list and passes calls the right instance (by its ID). Instances cannot change. They can only be removed and added which trigger ZOCP MOD calls. (Not sure if that's sufficient!)

Here's some code:

class ZOCPParameter(object):
    """
    Wrapper class for parameters used through ZOCP
    """
    _id = 0                 # id counter

    def __init__(self, znode, value, name, access, type_hint, signature, min=None, max=None, *args, **kwargs):
        self._value = value
        self._znode = znode                                 # reference to the ZOCPNode instance
        self.name = name                                    # name of the parameter
        self.min =  min                                     # minimum value of the parameter (optional)
        self.max = max                                      # maximum value of the parameter (optional)
        self.access = access                                # description of access methods (Read,Write,signal Emitter,Signal receiver)
        self.type_hint = type_hint                          # a hint of the type of data
        self.signature = signature                          # signature describing the parameter in memory
        # get ourselves an id
        self.sig_id = ZOCPParameter._id                     # the id of the parameter (needed for referencing to other nodes)
        ZOCPParameter._id += 1
        # in case we're an emitter overwrite the set method
        if 'e' in self.access:
            self.set = self._set_emit
        self._subscribers = {}                              # dictionary containing peer receivers for emitted signals in case we're an emitter
        self._subscriptions = {}                            # dictionary containing peer emitters for receiver in case we are a signal receiver

    def _set_emit(self, value):
        """
        Set and emit value as a signal
        """ 
        self._value = value
        self._znode.emit_signal(self.sig_id, self._to_bytes)

    def get(self):
        return self._value

    def set(self, value):
        self._value = value

    def signal_subscribe_emitter(self, emitter, peer, receiver):
        pass

    def signal_subscribe_receiver(self, receiver, peer, emitter):
        pass

    def _to_bytes(self):
        """
        converts value to an array of bytes
        """
        return struct.pack(self.signature, self.value)

    def _to_dict(self):
        """
        Converts this parameter to representing dictionary
        """
        d = {}
        d['name'] = self.name
        if self.min:
            d['min'] = self.min
        if self.max:
            d['max'] = self.max
        d['access'] = self.access
        d['typeHint'] = self.type_hint
        d['sig'] = self.signature
        d['sig_id'] = self.sig_id
        d['subscribers'] = self._subscribers
        d['subscriptions'] = self._subscriptions
        return d

    value = property(get, set)
sphaero commented 9 years ago

Started a new branch https://github.com/sphaero/pyZOCP/tree/capability_refactor

sphaero commented 9 years ago

I also made some changes to the node editor. However it takes some more changes to go from signal names to signal ids. https://github.com/sphaero/pyZNodeEditor/tree/capability_refactor

sphaero commented 9 years ago

Nodeeditor now works with capability reorganisation although I reckon it should be coded different.