0

So I've done some googling and other site, my console is filled with all these warnings. Just a bit unsure how to fix this method overall. Likewise I hope my code isn't vague since I'm not too sure if the code here is enough. Regardless here is the issue. I have been trying to integrate a Perl script within my module for a while. The script itself works but not in the module. The error message for when I try to run it is this method:

 sub generate_pillars_shape{
    my $d=$_[4];
    @X_values=$_[0]/$d..$_[1]/$d;
    @X_values=map{$_*$d} @X_values;
    @Y_values=$_[2]/$d..$_[3]/$d;
    @Y_values=map{$_*$d} @Y_values;
    for $i (0..$#X_values){
    @Y=(@Y,@Y_values);
    for $j (0..$#Y_values){
        $X[$i*($#Y_values+1)+$j]= $X_values[$i];
    }
    }
    return (\@X,\@Y);
}

The entire code consists of this:

use 5.010;
use Math::Trig ':radial';
use Math::Trig;
use List::Util qw(max min);

my $min_X=0;
my $max_X=60;
my $min_Y=0;
my $max_Y=60;
my $distance=10;
my @X_values;
my @Y_values;
my $i;
my $j;

#The minimum angle from horizontl your printer can make, in degrees
my $min_angle= 30;

#Ignore the next line, it is not an input parame
my @Z;
my ($X_ref,$Y_ref)= generate_pillars_shape($min_X,$max_X,$min_Y,$max_Y,$distance);my @X=@$X_ref;my @Y=@{$Y_ref};
for my $i (0..$#X){
    $Z[$i]=20;#The function that defined the height of each point. This setting wil give you a flat roof. For a more advanced tree, try:
    #$Z[$i]=-0.01*$X[$i]**2+0.2*$Y[$i]-0.005*$Y[$i]**2+20;
}

#End of input parameters.

my $min_radian = deg2rad($min_angle);
my $b = tan($min_radian);
@Z=map{$_/$b} @Z;



while ($#X>0){
  my ($I,$J)=find_min_dist(\@X,\@Y,\@Z);
  my ($X_branch,$Y_branch,$Z_branch)=find_branch($X[$I],$Y[$I],$Z[$I],$X[$J],$Y[$J],$Z[$J]);
  my @X_list= ($X_branch,$X[$I],$X[$J]);
  my @Y_list= ($Y_branch,$Y[$I],$Y[$J]);
  my @Z_list= ($Z_branch,$Z[$I],$Z[$J]);
  for my $j (0..$#Y_list){
      if (abs($X_list[my $j]) < 0.001){
      $X_list[$j]=0;
      }
      if (abs($Y_list[my $j]) < 0.001){
      $Y_list[$j]=0;
      }
      if (abs($Z_list[my $j]) < 0.001){
      $Z_list[$J]=0;
      }
  }
  branch (\@X_list,\@Y_list,\@Z_list);
  splice (@X,$I,1,$X_branch);
  splice (@X,$J,1);
  splice (@Y,$I,1,$Y_branch);
  splice (@Y,$J,1);
  splice (@Z,$I,1,$Z_branch);
  splice (@Z,$J,1);
}

sub generate_pillars_shape{
    my $d=$_[4];
    @X_values=$_[0]/$d..$_[1]/$d;
    @X_values=map{$_*$d} @X_values;
    @Y_values=$_[2]/$d..$_[3]/$d;
    @Y_values=map{$_*$d} @Y_values;
    for $i (0..$#X_values){
    @Y=(@Y,@Y_values);
    for $j (0..$#Y_values){
        $X[$i*($#Y_values+1)+$j]= $X_values[$i];
    }
    }
    return (\@X,\@Y);
}

sub branch{
  my @X=@{ $_[0] };
  my @Y=@{ $_[1] };
  my @Z=@{ $_[2] };
  @Z=map{$_*$b}@Z;
  for my $i (1..$#X){
    my ($rho, $theta, $phi) = cartesian_to_spherical($X[$i]-$X[0],$Y[$i]-$Y[0],$Z[$i]-$Z[0]);
    $phi = rad2deg($phi);
    if (abs($phi)<0.001){$phi=0;}
    $theta = rad2deg($theta)+90;
    if (abs($theta)<0.001){$theta=0;}
    if (abs($rho)>0.001){}
  }
}

sub find_min_dist{
  my @X=@{ $_[0] };
  my @Y=@{ $_[1] };
  my @Z=@{ $_[2] };
  my $min_dist=($X[0]-$X[1])**2+($Y[0]-$Y[1])**2+($Z[0]-$Z[1])**2;
  my $max_Z=$Z[0];
  my $I=0;
  my $J=1;
  for my $i (1..$#Z){
      if ($Z[$i]>=$max_Z){
      $max_Z=$Z[$i];
      my $I=$i;}
  }
  for my $j (0..$#X){
    if ($j!=$I){
      my $dist=(($X[$I]-$X[$j])**2+($Y[$I]-$Y[$j])**2+($Z[$I]-$Z[$j])**2);
      if ($min_dist>$dist){
    $min_dist=$dist;
    my $J=$j;
      }}}
  return ($I,$J);
}

sub find_branch{
  my $X1=$_[0];
  my $Y1=$_[1];
  my $Z1=$_[2];
  my $X2=$_[3];
  my $Y2=$_[4];
  my $Z2=$_[5];
  my $rXY=sqrt(($X1-$X2)**2+($Y1-$Y2)**2);
  if (abs($Z1-$Z2) < $rXY) {
    my $Z_branch=($Z1+$Z2-$rXY)/2;
    my $a=($Z1-$Z_branch)/$rXY;
    my $X_branch=(1-$a)*$X1+$a*$X2;
    my $Y_branch=(1-$a)*$Y1+$a*$Y2;
  }
  elsif ($Z1 < $Z2) {
    my $X_branch=$X1;
    my $Y_branch=$Y1;
    my $Z_branch=$Z1;
  }
  else {
    my $X_branch=$X2;
    my $Y_branch=$Y2;
    my $Z_branch=$Z2;
  }
  return my($X_branch,$Y_branch,$Z_branch);
}

I hope that explains the general situation, any help would be appreciated. Thanks.

John Kugelman
  • 349,597
  • 67
  • 533
  • 578
User0110101
  • 119
  • 3
  • 12
  • I get 4970 warnings (that's an exact number, not just meant to say "a lot") when I run your code, and none of them is about an uninitialized value in a division... Also, please add your imports to your code (I'm guessing you are using `Math::Trig` for instance?). – Dada Mar 19 '19 at 09:35
  • 2
    Don't put `my` before every assignement. You should use `my` once per variable. For instance `$X_list[my $j]` should be `$X_list[$j]`, `return my($X_branch,$Y_branch,$Z_branch);` should be `return ($X_branch,$Y_branch,$Z_branch);`. When you write `my $x`, you redefine `$x` and it becomes undefined... – Dada Mar 19 '19 at 09:39
  • Sorry @Dada, that is correct and will add the now. – User0110101 Mar 19 '19 at 09:41
  • 1
    And using `my` within an `if`-`else` block limits the scope of the defined variable to this block. – dgw Mar 19 '19 at 09:49
  • So should I just declare them all at the start? – User0110101 Mar 19 '19 at 10:02
  • No. The variables should be declared in the smallest possible scope. For example within a subroutine. – dgw Mar 19 '19 at 10:08
  • Ah okay, I have declared them now but still getting the division error. It just says illegal division by 0 in that generate pillar shape. – User0110101 Mar 19 '19 at 10:12

1 Answers1

5

I can't reproduce your error. However, there are a lot of errors in your code, so let me go through them, and hopefully this will help you find your actual mistake.

First, variables should be defined in the smallest possible scope: if a variable is used only within a function, it should be defined within this function. If a variable is used only in a for loop, it should be defined within this loop. In that spirit, you should remove

my $i;
my $j;

at the begining of your code. Also, keep in mind that my declares a lexical variable visible only in the current scope (ie. you can use it only in the current block). For instance,

else {
    my $X_branch=$X2;
    my $Y_branch=$Y2;
    my $Z_branch=$Z2;
}

declares 3 variables that don't exist after the else block.

Second, my declares a new variable, and should therefore be used only once per variable. If you write

my $x = 5;
return my $x;

The first line declares a variable $x, and set it to 5. The second line declares a new variable $x (thus shadowing the old one), whose value is undef, and returns it. What you want to write instead is:

my $x = 5;
return $x;

Now let me go through your code to point out a few mistakes/improvements:

  • $X_list[my $j] should be $X_list[$j] (as per the beginning of this answer).

  • Still in find_branch, you have an issue with the scope of the variables you defined (see the beginning of my answer). You should have something like:

    my ($X_branch, $Y_branch, $Z_branch);
    if (abs($Z1-$Z2) < $rXY) {
        $Z_branch=($Z1+$Z2-$rXY)/2;
        my $a=($Z1-$Z_branch)/$rXY;
        $X_branch=(1-$a)*$X1+$a*$X2;
        $Y_branch=(1-$a)*$Y1+$a*$Y2;
    }
    elsif ($Z1 < $Z2) {
        $X_branch=$X1;
        $Y_branch=$Y1;
        $Z_branch=$Z1;
    }
    else {
        $X_branch=$X2;
        $Y_branch=$Y2;
        $Z_branch=$Z2;
    }
    return ($X_branch,$Y_branch,$Z_branch);
    

    This two corrections should silence every warnings. However, I suspect there are more things going wrong in your code.

  • In find_min_dist, you should not write my $I = $i and my $J = $j but rather $I = $i and $J = $j (still the same scoping issue).

  • Your sub branch doesn't do anything: you compute some $rho, $theta and $phi, but you don't return them (and you don't modify the arguments either).

  • In generate_pillars_shape, @X_values, @Y_values, @X, @Y should all be locally declared with my. Also, you can initialize @X_values with @X_values = grep { $_ % $d == 0 } $_[0] .. $_[1] (same for @Y_values), which I find more readable that what you wrote.

  • You should put your code in functions or code blocks ({ ... }) to use proper scoping: while it can make sense to have $min_X, $max_X, $min_Y, $max_Y,and $distance as global variables, you definitely don't want to have $min_radian or $b defined everywhere in your file.

  • Don't use $a or $b (they are special variables, used by sort; you don't want to mess with them (see this question for instance)).

  • Additionally, just for clarity, in your sub find_branch, you can be a bit more compact to retrieve the arguments:

      my ($X1, $Y1, $Z1, $X2, $Y2, $Z2) = @_;
    

I'm fairly convinced that there are other issues with your code. Please tell us what you are trying to do and what each function is supposed to do if you want more help.

Dada
  • 6,313
  • 7
  • 24
  • 43