CollisionGroup not updating value

[ScheiklP]

Hi! :)

I am trying to change the collision group of a collision model during runtime. However, the values are not updated correctly, and the .array() method returns weird values.

Example to reproduce:

import numpy as np
import Sofa
import Sofa.Core
import Sofa.Simulation

class SimulationHandler:
    def __init__(self):
        self.root_node = Sofa.Core.Node("root")
        self.nodes = createScene(self.root_node)
        Sofa.Simulation.init(self.root_node)

    def step(self):
        Sofa.Simulation.animate(self.root_node, self.root_node.getDt())

def createScene(root):
    plugins = [
        "Sofa.Component.Collision.Detection.Algorithm",
        "Sofa.Component.Collision.Detection.Intersection",
    ]
    for plugin in plugins:
        root.addObject("RequiredPlugin", pluginName=plugin, name=plugin)

    root.addObject("DefaultAnimationLoop")
    root.addObject("DefaultPipeline")
    root.addObject("BruteForceBroadPhase")
    root.addObject("BVHNarrowPhase")
    root.addObject("DefaultContactManager")
    root.addObject(
        "LocalMinDistance",
        alarmDistance=5.0,
        contactDistance=0.5,
    )

    node_1 = root.addChild("child_1")
    node_1.addObject("MechanicalObject", position=[0, 0, 0] * 5)
    node_1.addObject("PointCollisionModel", group=0)

    node_2 = root.addChild("child_2")
    node_2.addObject("MechanicalObject", position=[1, 1, 1] * 5)
    node_2.addObject("PointCollisionModel", group=1)

    return node_1, node_2

if __name__ == "__main__":
    simulation = SimulationHandler()

    simulation.step()
    print(f"First step: {simulation.nodes[0].PointCollisionModel.group.value=}")
    print(f"First step: {simulation.nodes[1].PointCollisionModel.group.value=}")
    print(f"Array: {simulation.nodes[0].PointCollisionModel.group.array()=}")
    print(f"Array: {simulation.nodes[1].PointCollisionModel.group.array()=}")
    print("-----")
    simulation.nodes[0].PointCollisionModel.group.value[:] = np.array([8])
    simulation.nodes[1].PointCollisionModel.group.value[:] = np.array([9])
    print(f"After setting: {simulation.nodes[0].PointCollisionModel.group.value=}")
    print(f"After setting: {simulation.nodes[1].PointCollisionModel.group.value=}")
    print(f"Array: {simulation.nodes[0].PointCollisionModel.group.array()=}")
    print(f"Array: {simulation.nodes[1].PointCollisionModel.group.array()=}")
    simulation.step()
    print("-----")
    print(f"Second step: {simulation.nodes[0].PointCollisionModel.group.value=}")
    print(f"Second step: {simulation.nodes[1].PointCollisionModel.group.value=}")
    print(f"Array: {simulation.nodes[0].PointCollisionModel.group.array()=}")
    print(f"Array: {simulation.nodes[1].PointCollisionModel.group.array()=}")

Output:

First step: simulation.nodes[0].PointCollisionModel.group.value=[[0]]
First step: simulation.nodes[1].PointCollisionModel.group.value=[[1]]
Array: simulation.nodes[0].PointCollisionModel.group.array()=array([1073741824], dtype=int32)
Array: simulation.nodes[1].PointCollisionModel.group.array()=array([1073741824], dtype=int32)
-----
After setting: simulation.nodes[0].PointCollisionModel.group.value=[[0]]
After setting: simulation.nodes[1].PointCollisionModel.group.value=[[1]]
Array: simulation.nodes[0].PointCollisionModel.group.array()=array([1073741824], dtype=int32)
Array: simulation.nodes[1].PointCollisionModel.group.array()=array([1073741824], dtype=int32)
-----
Second step: simulation.nodes[0].PointCollisionModel.group.value=[[0]]
Second step: simulation.nodes[1].PointCollisionModel.group.value=[[1]]
Array: simulation.nodes[0].PointCollisionModel.group.array()=array([1073741824], dtype=int32)
Array: simulation.nodes[1].PointCollisionModel.group.array()=array([1073741824], dtype=int32)

Any ideas what might be wrong here?

Sofa commit: https://github.com/sofa-framework/sofa/commit/9a0d4b9f3c1f6f6b508704f8b89d4304794e6291 SP3 commit: 5a7371616fe8914530d44bf25ea6b724a6b1a08e

Cheers, Paul

[hugtalbot]

Hey @ScheiklP

I tested a very simple xml scenario where I set in the XML the group datafield of the CollisionModels and then change one and it does take the change into account (all initially group=0 and then I change for one object group=1 and this object goes through the other ones)

Maybe something related to the binding between the python array and the C++ Data which is a std::set<int> @damienmarchal could maybe help here..

Thanks for reporting it anyway @ScheiklP :+1:

[hugtalbot]

Sorry my bad, it's indeed not working :+1:

[ScheiklP]

@hugtalbot So it's more a Sofa than a SofaPython3 issue? Should I open another issue there?

[hugtalbot]

Yes :+1:

[hugtalbot]

My first feeling would be that this is due to the 2-step broad-narrow phases : the data group concerns both but when modifying the group of the CollisionModel of your object, it's actually used for the narrow phase (Proximity) while the group information should be propagated to all phases. In addition I guess that this group even gets overwritten (in the GUI after changing it, and reopening it, the value is the initial one)

Let's investigate

[alxbilger]

I did

self.assertEqual(node.PointCollisionModel.group.value, [[0]])
node.PointCollisionModel.group = [8]
self.assertEqual(node.PointCollisionModel.group.value, [[8]])

and the result is

self.assertEqual(node.PointCollisionModel.group.value, [[8]])
AssertionError: Lists differ: [[0], [8]] != [[8]]

It seems that setting a value in a set insert the new value, but does not remove the old one. I think it is due to https://github.com/sofa-framework/sofa/blob/918cd66008f586575c92c1e068f5c267a952b936/Sofa/framework/DefaultType/src/sofa/defaulttype/typeinfo/models/SetTypeInfo.h#L113.

I think this discussion should involve @damienmarchal.

[alxbilger]

Here is the unit test:

# coding: utf8

import Sofa.Core
import Sofa.Components
import unittest
import numpy as np

class Test(unittest.TestCase):
    def test_setting_collision_group(self):
        root = Sofa.Core.Node("rootNode")
        plugins = [
            "Sofa.Component.Collision.Detection.Algorithm",
            "Sofa.Component.Collision.Detection.Intersection",
        ]
        for plugin in plugins:
            root.addObject("RequiredPlugin", pluginName=plugin, name=plugin)

        root.addObject("DefaultAnimationLoop")
        root.addObject("DefaultPipeline")
        root.addObject("BruteForceBroadPhase")
        root.addObject("BVHNarrowPhase")
        root.addObject("DefaultContactManager")
        root.addObject("LocalMinDistance", alarmDistance=5.0, contactDistance=0.5)

        node = root.addChild("child")
        node.addObject("MechanicalObject", position=[0, 0, 0] * 5)
        node.addObject("PointCollisionModel", group=0)

        Sofa.Simulation.init(root)
        Sofa.Simulation.animate(root, root.getDt())

        self.assertEqual(node.PointCollisionModel.group.value, [[0]])
        node.PointCollisionModel.group = [8]
        self.assertEqual(node.PointCollisionModel.group.value, [[8]])

[damienmarchal]

Thanks for having added me in this nicely documented issues with code to code & paste for testing things.

[damienmarchal]

Ok, so after quick investigation, there are several issues related to how sets are exposed to python related to what @alxbilger pointed. The real problem is that SetTypeinfo is "hijacking" the typeinfo API for indexable container. This is very visible in the SetTypeInfo::setSize() that actually clears the set as well as using setValue(..., index, ) which is ignoring the index and insert the data.

The consequence of that is that sets are considered as an indexable container by the python binding but as they are not really fullfilling what could be expected from an indexable container things goes badly wrong.

eg:

• when we get the value from a set it get interpreted as a two dimmensional array when converted to python. This is why "value" returns [[0],[1],[2]] instead of [0,1,2] which is a consistency issue when setting the value which expect [1,2,3] and not [[1],[2],[3]]

• when we set the value from python. The codepath is the one for a indexed container, this code path is using setSize() to initialize data only if the current set size and the new ones mis-matches (which is not the case in the provided example). The consequence is that setSize() is not called at all, which is perfectly ok for a resizable container but not for a set.

Quick fix can be done in the python binding by calling setSize() everytime we do setValue, this will force the set to be cleared, this fix only solve the reported issue but not the other problems (the conversion when we get the data).

A much clean way of fixing that is to add complete support for "Unique Key containers" the typeinfo system with specific dedicated API. This probably means adding feature in AbstractTypeInfo.

[hugtalbot]

Thanks for the feedback @damienmarchal

[ScheiklP]

@damienmarchal Thanks a lot! :) Could you point me to the relevant code for the quick fix? :sweat_smile:

[damienmarchal]

@damienmarchal Thanks a lot! :) Could you point me to the relevant code for the quick fix? sweat_smile

In PythonFactory.cpp you need to comment the pointed line...

template<class DestType>
void copyFromListOf(BaseData& d, const AbstractTypeInfo& nfo, const py::list& l)
{
    /// Check if the data is a single dimmension or not.
    py::buffer_info dstinfo = toBufferInfo(d);

    if(dstinfo.ndim>2)
        throw py::index_error("Invalid number of dimension only 1 or 2 dimensions are supported).");

    if(dstinfo.ndim==1)
    {
        void* ptr = d.beginEditVoidPtr();
        if( size_t(dstinfo.shape[0]) != l.size())        // THIS IS THE LINE TO COMMENT TO FORCE CLEARING of a set<- 
            nfo.setSize(ptr, l.size());

        for(size_t i=0;i<l.size();++i)
        {
            copyFromListOf<DestType>(nfo, ptr, i, l[i]);
        }
        d.endEditVoidPtr();
        return;
    }
    /// ...
}

NB: I tried to fix that the right way... but working in TypeInfo.h is such a source of pain because of its misdesign that I will probably give-up soon.

[hugtalbot]

Did this quick-dirty fix solve your issue @ScheiklP ?

[ScheiklP]

I did not try it yet. I was hoping for a miracle in https://github.com/sofa-framework/sofa/pull/3851 :D