PointCloud For.Parallel Crash

Hi,

I would like to iterate through pointcloud in a parrallel for loop. But this method crashes.
I am checking if each point is between lowest and highest box corners for simple box crop.
This works without for.parallel.

Where do I make a mistake considering crashing?

public PointCloud CropCloudParallel(PointCloud cloud, Point3d Min, Point3d Max, bool inverse = false) {



    PointCloud croppedCloud = new PointCloud();


    System.Threading.Tasks.Parallel.For(0, cloud.Count, i => {

        var p = cloud[i].Location;

        bool flag = (Min.X < p.X) && (Max.X > p.X) &&
            (Min.Y < p.Y) && (Max.Y > p.Y) &&
            (Min.Z < p.Z) && (Max.Z > p.Z);

        if (inverse)
            flag = !flag;

        if (flag) {
            
            if (cloud.ContainsNormals) {
               croppedCloud.Add(item.Location, item.Normal,item.Color);
            } else {
                croppedCloud.Add(cloud[i].Location, cloud[i].Color);
            }
        }

    });

    return croppedCloud;

}

Probably PointCloud.Add is not threadsafe. Use a threadsafe collection inside the parallel loop, or collect results in a Boolean array. Then construct the pointcloud based on that.

2 Likes

Thanks it worked with bool array.
What is the second option when you say threadsafe collection? I.e. adding points to a List ?

public PointCloud CropCloudParallel(PointCloud cloud, Point3d Min, Point3d Max, bool inverse = false) {



            PointCloud croppedCloud = new PointCloud();
            bool[] f = new bool[cloud.Count];

            System.Threading.Tasks.Parallel.For(0, cloud.Count, i => {

                var p = cloud[i].Location;

                bool flag = (Min.X < p.X) && (Max.X > p.X) &&
                    (Min.Y < p.Y) && (Max.Y > p.Y) &&
                    (Min.Z < p.Z) && (Max.Z > p.Z);

                if (inverse)
                    flag = !flag;

                if (flag) {
                    f[i] = true;
                }

            });

            for(int i = 0; i< f.Length; i++) {
                if(f[i])
                    if (cloud.ContainsNormals) {
                        croppedCloud.Add(cloud[i].Location, cloud[i].Normal, cloud[i].Color);
                    } else {
                        croppedCloud.Add(cloud[i].Location, cloud[i].Color);
                    }
            }

            return croppedCloud;

        }

Threadsafe collections are ConcurrentBag or ConcurrentDictionary for example. But I think the boolean array solution is more simple and more performant, there really is no need to use a Concurrent* collection. I think these use a thread lock internally, that hurts performance. With the boolean array there is no need to use locks.

Another, even more performant way to do this is with an RTree, especially if you are doing this kind of queries repeatedly on the same PointCloud.

https://developer.rhino3d.com/api/RhinoCommon/html/M_Rhino_Geometry_RTree_CreatePointCloudTree.htm

https://developer.rhino3d.com/api/RhinoCommon/html/M_Rhino_Geometry_RTree_Search_1.htm

Thank you.

Ah yes the RTree bounding-box search, totally forgot about this.
I was using RTree for bounding box search before.

1 - But how do you iterate RTree when you have a PointCloud and a list of boxes?
Set A - points, Set B - boxes.

Before I was using RTree only for closest point search like the code below with the Callback.
2 - Should I follow the same procedure by adding one more point with a larger bounding box?

3 - And how do you use the RTree for rotated boxes, because now I each time orient pointcloud to xy plane to perform min max inclusion test.

       RTree RTreeMeshBBoxes = new RTree();

        //////////////////////////////////////////////////////////////////////////////
        //Get BoundingBoxes, inflate them add them to dictionary.
        //////////////////////////////////////////////////////////////////////////////
        Dictionary<int, BoundingBox> boundingBoxes = new Dictionary<int, BoundingBox>();

        //////////////////////////////////////////////////////////////////////////////
        //Create RTree (1) Insert bounding boxes + indices (2) Perform search
        //////////////////////////////////////////////////////////////////////////////

        int c = 0;
        foreach (var kv in elements) {
            var bbox = kv.Value.bbox;
            bbox.Inflate(1.1);
            boundingBoxes.Add(c, bbox);
            c++;
        }

        //Insert BoundingBoxes to RTree
        for (int i = 0; i < boundingBoxes.Count; i++)
            RTreeMeshBBoxes.Insert(boundingBoxes[i], i);


        //Perform search
        for (int i = 0; i < RTreeMeshBBoxes.Count - 1; i++) {
            int? index = i;//Declare nullable values
            bool ts = RTreeMeshBBoxes.Search(boundingBoxes[i], SearchCallback, index);
        }

This is correct. Add as a function in general on most things is not a threadsafe operation. Only the concurrent collections that Menno refers to should be considered thread safe for adding items to any sort of collection.

2 Likes

Thanks @stevebaer as well for a reply :wink: