Finding minimum and maximum component values from a list of points in C#

unhandled

#1

Hi there,

That is my 3rd trial in C#, so practically I´m a noob.

The idea of this component is that from a list of given points I can get its minimum and maximum component value.

I manage to do it, but I think it can be better optimised than that of its current form.image

How can I make this code more efficient ?

    //List of points' coordinates components
    List<double> xList = new List<double>();
    List<double> yList = new List<double>();
    List<double> zList = new List<double>();

    //Points Components' extractor
    foreach(Point3d element in nPoints){
      xList.Add(element.X);
      yList.Add(element.Y);
      zList.Add(element.Z);
    }

    //Export answers
    xMin = xList.Min();
    xMax = xList.Max();

    yMin = yList.Min();
    yMax = yList.Max();

    zMin = zList.Min();
    zMax = zList.Max();

Thank you in advance.


(qythium) #2

Your code looks alright- it could be a little less verbose ( xMin = nPoints.Min(p => p.X); etc.). but that shouldn’t affect the actual performance much.

I suspect most of the overhead you are seeing comes from the GH_Point to Point3d casting that happens at the C# script input parameter. What happens when you change the input type hint from Point3d to System.Object?

(Attach a definition with internalized params so others can try benchmarking with your data :slight_smile: )


(David Rutten) #3

Note that the first time a C# component runs the time required to compile the code is included in the performance measure. Press F5 a number of times to get a better feel for long it really takes.


(Michael Pryor) #5

To add onto what @DavidRutten is saying. Also note that your c# component is doing more work than the deconstruct point component (it is also finding mins and max which requires a list search, where as the deconstruct point component is just outputting point properties). So it is not really an even comparison.


#6

@Michael_Pryor how to flag the image ??


#7

@qythium Thank you, that is exactly what I was hoping to know ?

By the way, is there a place you know where I can know better about C# for a total noob like me ?


#8

By the way, I´m aware that my component is doing more work than the collection of GH native components combined, but my idea is that if I can’t script a component that can do the same work in less time, then may be the idea of scripting is useless, so the other way is to improve my scripting abilities


#9

Would be possible speed it up by the use of math.min and math.max inside the foreach loop instead of using linq?


#10

@DavidRutten followed your advise before changing the script, my component scores between 105-140 ms, while the others combines between 20-40ms

@Baris I will work on it, just tell me how. Please.


(Michael Pryor) #11

Flag the image? Not sure what you are talking about.


#12

about that


(Michael Pryor) #13

There is no image, I removed the post because I replied to the wrong person. The post is same as what I wrote.


(David Rutten) #14

assuming your nPoints collection has a Length or a Count property, I’d do it like this. But there’s quite a few optimisations in there I wouldn’t bother with unless this was critical code.

//List of points' coordinates components
if (nPoints.Count == 0)
  return;

double[] x = new double[nPoint.Count];
double[] y = new double[nPoint.Count];
double[] z = new double[nPoint.Count];

double xMin = nPoints[0].X;
double xMax = xMin;
double yMin = nPoints[0].Y;
double yMax = yMin;
double zMin = nPoints[0].Z;
double zMax = zMin;

for (int i = 1; i < nPoints.Count; i++)
{
  double x = nPoints[i].X;
  if (x < xMin) 
    xMin = x;
  else if (x > xMax) 
    xMax = x;

  double y = nPoints[i].Y;
  if (y < yMin) 
    yMin = y;
  else if (y > yMax) 
    yMax = y;

  double z = nPoints[i].Z;
  if (z < zMin) 
    zMin = z;
  else if (z > zMax) 
    zMax = z;
}

#15

After few corrections to the parts I could understand, I got these messages

Error (CS0019): Operator '<' cannot be applied to operands of type 'double' and 'object' (line 88)
Error (CS0019): Operator '>' cannot be applied to operands of type 'double' and 'object' (line 90)
Error (CS0136): A local variable named 'y' cannot be declared in this scope because it would give a different meaning to 'y', which is already used in a 'parent or current' scope to denote something else (line 93)
Error (CS0019): Operator '<' cannot be applied to operands of type 'double' and 'object' (line 94)
Error (CS0019): Operator '>' cannot be applied to operands of type 'double' and 'object' (line 96)
Error (CS0136): A local variable named 'z' cannot be declared in this scope because it would give a different meaning to 'z', which is already used in a 'parent or current' scope to denote something else (line 99)
Error (CS0019): Operator '<' cannot be applied to operands of type 'double' and 'object' (line 100)
Error (CS0019): Operator '>' cannot be applied to operands of type 'double' and 'object' (line 102)

(David Rutten) #16

I was using xMin etc. as local variables. If they are defined as function out parameters because the are component outputs, you’ll have to switch to some other internal variable. Out parameters are always of type object, meaning you cannot use them as you would more specific variables.


(qythium) #17

@Nader_Belal If you are new to programming in general, it might be good to keep in mind the pitfalls of premature optimization… Most of the time the fractions of a second that you shave off a script is totally insignificant compared to the effort spent optimizing in the first place. And then subsequently wrangling with the code structure, which tends to be more rigid and hard to understand at a glance.

That’s why I would prioritize readability first of all - using foreach statements and LINQ expressions may have a slight performance cost, but their intent is more immediately obvious to the reader, and there is no chance of off-by-one errors dealing with indices -

xMin = nPoints.Min(p => p.X); 
xMax = nPoints.Max(p => p.X);
yMin = nPoints.Min(p => p.Y);
...

Only when there is a critical need to speed things up, e.g. huge data sets, need for real-time interactivity, should you start looking at making optimizations to existing code.


(qythium) #18

I would say the main advantage of scripting in Grasshopper context is not the speed, but the flexibility of being able to do more complex kinds of iteration logic and geometry operations, that are simply not possible with native components or require lots of bizarre contortions to achieve

Just for fun, I had a go at recreating your definition, and it turns out @DavidRutten’s optimized algorithm performs about 10-15% faster than the original or the LINQ approach on 100,000 points

Removing the type hint and doing casting within the script takes off an extra 20 ms or so across the board


point-minmax-optimization.gh (9.5 KB)


(Vicente Soler) #19

Alternatively, you can use a bounding box object:

   BoundingBox box = new BoundingBox(nPoints);
   double xMin = box.Min.X;
   double yMin = box.Min.Y;
   double zMin = box.Min.Z;
   double xMax = box.Max.X;
   double yMax = box.Max.Y;
   double zMax = box.Max.Z;

(Vicente Soler) #20

And if you’d like to see what the bounding box constructor is doing behind the scenes:

/// <summary>
/// Constructs a boundingbox from a collection of points.
/// </summary>
/// <param name="points">Points to include in the boundingbox.</param>
public BoundingBox(System.Collections.Generic.IEnumerable<Point3d> points)
  : this()
{
  bool first = true;
  foreach (Point3d pt in points)
  {
    if (first)
    {
      m_min = pt;
      m_max = pt;
      first = false;
    }
    else
    {
      if (m_min.m_x > pt.m_x)
        m_min.m_x = pt.m_x;
      if (m_min.m_y > pt.m_y)
        m_min.m_y = pt.m_y;
      if (m_min.m_z > pt.m_z)
        m_min.m_z = pt.m_z;

      if (m_max.m_x < pt.m_x)
        m_max.m_x = pt.m_x;
      if (m_max.m_y < pt.m_y)
        m_max.m_y = pt.m_y;
      if (m_max.m_z < pt.m_z)
        m_max.m_z = pt.m_z;
    }
  }
}

You can find the code here.


#21

Thank you all,

@visose’s script for bounding box have resulted in, the most optimised script, either in script length, clarity, maintenance , or time of execution followed by @DavidRutten which with his example I have learnt a lot.

@qythium

  1. You were right about premature optimisation. I will keep it in mind.
  2. I tried to remove type hint and I what I have got is error messages, so how should it be done ?? Just to know.

@All,

  1. Just an observation, Vicent’s code was the most optimised code when there were 4 C# active & connect components on Grasshopper’s canvas, but when I have started to disconnect all components except that of Vicent’s, the numbers started to rise … why is that ??? :astonished::astonished::astonished:

  2. Can anyone advise me something to start consulting C# tutorials for a total noob, written for normal people, all I can find are tutorials for people who have started to understand what this language is about … :tired_face:

And thank you, you have been of great help