Spurious `undefined' values and conversion issues

sf_ashley has asked for the wisdom of the Perl Monks concerning the following question:

Dear Monks,

Apologies for the long nature of this post. Unfortunately, I couldn't find a more succinct way of posing the following problem. I am the midst of writing a reasonably voluminous code to perform some reasonably straight-forward data analysis. One part of the code involves 'rebinning' spectra where the bin width of a spectrum is linearly interpolated to a format defined by the user (and is of a greater width than the original format). A couple of errors arise, that I do not know how to deal with and were problems that I would like to iron out. The subroutine code is given below, commented as appropriate and the error messages are as follows:

Firstly, I get the following warnings in this order: uninitialized value in addition on line 1153, unin. val in multiplication (line 1158), unin. val in subtraction (line 1162), unin. val. in numeric ge (>=) in line 1148. I also get a host of Invalid conversion warnings at line 1188, which I'll come to later. Now, @energy_before and @energy_rebin are defined as I have tested this by printing the values into an output file. Also, the values of @counts_rebin are also not entirely NULL, as I do end up getting a rebinned spectrum that is reasonably accurate. However, I don't know why it is coming up with these warnings and would like to be able to eradicate these. Any ideas as to what is causing this?

Secondly, when I say the spectra produced are reasonably accurate, the input file is definitely of a little-endian VAX format, as I had generated this earlier in the code. However, after rebinning, the spectra should be of a floating-point form. Now, when I have in line 1188, the expression 'V' for a VAX output, the code compiles but with the aforementioned warnings. If, I replace this with an 'f', I end up with a fatal error 'Modification of a read-only value attempted at line 1188'. Now, this has completely puzzled me. I think I understand that the invalid conversion should arise as I have non-integer values being parsed as integers. But I cannot see why there is a fatal exception here. Also, when I run this, I have extra digits in the invalid conversion warning, i.e. with 'V', it would just say invalid conversion in printf: at line 1188, <STDIN> line 15; with 'f' it would say (as an example) invalid conversion in printf: %\015 at line 1188 <STDIN> line 15. Why is this error arising? In the unpack command, is the data only treated as integers or does perl switch from handling these as integers to floats? Also, is there a specific format for a 4 byte little- or big-endian floating point value? I presumed this would be 'f'.

Anyway, I hope my errors aren't too trivial or are from bad programming form and suggestions regarding the aforementioned warnings/exceptions/errors would be very much appreciated. If further tests or information is required, please do not hesitate to ask.

Kind regards,

Steve

sub rebinner_interp{
#Rebinning portion of analysis code.  Define local variables used in t
+his subroutine
    my $idx, my $buf, my $i, my $j, my @calibcoeff;
    my @energy_rebin, my @values, my @energy_before;
    my $chn;
    my @acoeff, my @bcoeff, my @ccoeff;
#Note: $spec, %slope[] and %intercept[] are global variables that have
+ been defined earlier

#User to input their bin-width of choice, define new energy binning st
+ructure that is universal for all spectra 
    printf STDOUT "Please enter the energy width (in keV) of each bin\
+n";
    chomp (my $rebin_width = <STDIN>);
    for ($i=0; $i<16834; $i++){
    $energy_rebin[$i] = ($rebin_width * $i);
    }

#For each spectrum, $idx, read in original spectrum in VAX format to a
+rray @rawchannel
    for ($idx=0; $idx<=$spec; $idx++){
    my @rawchannel;
    open (INFILE, "<", "filename.spc") 
        or die "Can't find spectrum: filename.spc";
    while (read(INFILE, $buf, 4) == 4) {
        if (length($buf) > 0 && length ($buf) < 4){
        next;
        }
        else{
        push @rawchannel, scalar unpack('V',$buf);
        }
    }
    close INFILE;

#For $idx, determine the energy binning before from pre-determined cal
+ibration
    $chn = 0;
    $i = 1;
    for ($chn=0;$chn<16384;$chn++){
        if ($i == $calib_points){
        $energy_before[$chn] = ($slope{$idx}[$i]*${chn})+$intercept{$i
+dx}[$i];
        }
        if (($chn<=$channel{$idx}[$i])&&($i<$calib_points)){
        $energy_before[$chn] = ($slope{$idx}[$i]*$chn)+$intercept{$idx
+}[$i];
        }
        elsif (($chn > $channel{$idx}[$i]) && ($i < $calib_points)){
        $i += 1;  
        $energy_before[$chn] = ($slope{$idx}[$i]*$chn)+$intercept{$idx
+}[$i];  
        }
        
    }
#Originally, test script placed here to check the output of each @ener
+gy_rebin and @energy_before.  So the script should be fine up to here
+.

#Define @counts_rebin for each spectrum which will contain the number 
+of counts per channel for each rebinned spectrum 
    $j=0;
    my $k = 0;
    my @counts_rebin;
    my $value;
#Loops to generate rebinned spectrum.  First part intended to define a
+ value for $counts_rebin[$j] if undefined.  Then depending on the ord
+er add all or part of the bin as fit.  Note, from test $k is always l
+ess than 16382.
    for($j=0; $j<=16831; $j++){
        if(($energy_rebin[$k] >= $energy_before[$j])){ # l:1148
        if ((defined($counts_rebin[$k]))==0){
            $counts_rebin[$k]=0;
        }
        $value = $rawchannel[$j];
        $counts_rebin[$k] = ($counts_rebin[$k] + $value); # l:1153
        next;
        }
        if(($energy_rebin[$k] < $energy_before[$j]) && $j!=0){
        my $totcount = $rawchannel[$j];
        my $split1 = (($energy_before[$j]-$energy_rebin[$k])*$totcount
+)/($energy_before[$j]-$energy_before[${j}-1]); # l:1158
        if ((defined($counts_rebin[$k]))==0){
            $counts_rebin[$k]=0;
        }
        $counts_rebin[$k]=($counts_rebin[$k]+($totcount-$split1)); # l
+:1162
        $counts_rebin[$k+1]=$split1;
        $k+=1;
        next;
        }
        if(($energy_rebin[$k]<$energy_before[$j]) && $j==0){
        until ($energy_rebin[$k]>=$energy_before[$j]){
            $k += 1;
        }
        $value = $rawchannel[$j];
        $counts_rebin[$k] = $counts_rebin[$k] + $value;
        next;
        }
    }
#As $counts_rebin[$j] only partially filled, define the rest of the ar
+ray (up to 16382) as undefined 
    $j=0;
    for ($j=0;$j<=16381;$j++){
        if (defined($counts_rebin[$j])){
        next;
        }
        else{
        $counts_rebin[$j]=0;
        }
    }

#Output the file in a little-endian VAX format
    open (REBINOUT, ">", "filename.spr");
    binmode REBINOUT, ":raw";
    for ($i=0;$i<=16831;$i++){
        printf REBINOUT pack('V',$counts_rebin[$i]); # l: 1188
    }
    close REBINOUT;
    }
}
[download]

Comment on Spurious `undefined' values and conversion issues Download Code

Replies are listed 'Best First'.
Re: Spurious `undefined' values and conversion issues by almut (Canon) on May 30, 2008 at 01:06 UTC
`printf REBINOUT pack('V',$counts_rebin[$i]); # l: 1188` [download] Use `print` instead of `printf`. `printf` needs a format specification string as the first argument after the file handle REBINOUT. As you have not specified one, your packed data is being misinterpreted as a format string... Update: Your "uninitialized values" errors might have to do with that you're initializing `@energy_before` with a loop running up to 16384, but then are retrieving values from that array up to an index of 16831 ... (though I must admit I haven't looked into the code too deeply)	[reply] [d/l] [select]
Re^2: Spurious `undefined' values and conversion issues by sf_ashley (Sexton) on May 30, 2008 at 18:17 UTC
Many thanks for looking through the code and for your comments almut. Both the printf statement and the array limit were indeed a typographical errors (well, the latter was a spoonerism I suppose), which is a little frustrating, given the amount of time I have spent trying to find the error. Modifying the above, as appropriate, eradicated the errors/warnings. Again, many thanks for your help. Steve	[reply]
Re: Spurious `undefined' values and conversion issues by graff (Chancellor) on May 30, 2008 at 01:31 UTC
uninitialized value in addition on line 1153 `$counts_rebin[$k] = ($counts_rebin[$k] + $value); # l:1153` [download] Try writing that line this way, and that warning will go away (in this case, the warning is just a matter of style -- no impact on the final results): `$counts_rebin[$k] += $value; # l:1153` [download] unin. val in multiplication (line 1158) `my $totcount = $rawchannel[$j]; my $split1 = (($energy_before[$j]-$energy_rebin[$k])$totcount) +/($energy_before[$j]-$energy_before[${j}-1]); # l:1158` [download] That bit is in a for loop that goes up to "$j==16831" -- are you sure you have that many elements in @rawchannel? unin. val in subtraction (line 1162),* `$counts_rebin[$k]=($counts_rebin[$k]+($totcount-$split1)); # l +:1162` [download] Probably the same issue as the previous one, relating to $totcount (and @rawchannel), but I can't say for sure. unin. val. in numeric ge (>=) in line 1148 `if(($energy_rebin[$k] >= $energy_before[$j])){ # l:1148` [download] I think there may be something odd going on with how you are incrementing $k inside the main for loop ($j=0..16381) -- maybe it's getting incremented to far? But that's just a guess. Maybe the "until" loop at line #1168 should be done like this: `until ($k==@energy_rebin or $energy_rebin[$k]>=$energy_bef +ore[$j]){ $k += 1; }` [download] That would make sure that $k never points to an uninitialized offset in that array (but I don't know if it does the right thing as far as your algorithm is concerned). Modification of a read-only value attempted at line 1188 `printf REBINOUT pack('V',$counts_rebin[$i]); # l: 1188` [download] I really don't understand why you are using "printf" there -- it makes no sense to me. Wouldn't you rather take the value returned by "pack" and just "print" it? (update: the first item -- line #1153 -- is one of many examples where your coding style is more fortran-like rather than perlish (or even C-like), which I suppose is not unexpected for this sort of app... And the last item (printf) might be a symptom of C-centric style. Of course, you don't need to make it "more perlish" just for the sake of doing that, but increasing the use of some of the more compact perl idioms might be a good thing.)	[reply] [d/l] [select]
Re^2: Spurious `undefined' values and conversion issues by sf_ashley (Sexton) on May 30, 2008 at 18:36 UTC
Many thanks for your nice reply, graff. Regarding the uninitialized values, it is very much due to $j being too large (through a rather pesky typo, that had evaded my checking). The $k value should be fine as this should always be smaller compared to $j. However, now that you have brought it up, I should write an appropriate die command if $k >= $j. Also, your suggestion regarding += has been taken on board as this is more elegant, and also consistent with other parts of the code. In addition, replacing printf with print on line 1188, yielded correct spectra. Regarding perl idioms, I am still rather new to the game of programming with perl and am slowly finding my feet. However, I still very much have fortran thought processes! Again, many thanks for your response and for your suggestions. Steve	[reply]
Re: Spurious `undefined' values and conversion issues by oko1 (Deacon) on May 30, 2008 at 01:28 UTC
On line 1158: `my $split1 = (($energy_before[$j]-$energy_rebin[$k])*$totcount)/($ener +gy_before[$j]-$energy_before[${j}-1]);` [download] Note the "$energy_before[${j}-1]". What happens to that index when $j is 0? -- Human history becomes more and more a race between education and catastrophe. -- HG Wells	[reply] [d/l]
Re^2: Spurious `undefined' values and conversion issues by sf_ashley (Sexton) on May 30, 2008 at 18:25 UTC
Thanks for your suggestion, oko1. If $j=0, the if loop shouldn't run, as the logic condition given in the if statement is `if(($energy_rebin[$k] < $energy_before[$j]) && $j!=0){`. The problem for this case was that $j was too large due to a typograhical error in the 'for' loop and this yielded the undefined value problem. However, many thanks for looking through the code and for your suggestion. Steve	[reply] [d/l]
Re: Spurious `undefined' values and conversion issues by oko1 (Deacon) on May 30, 2008 at 01:46 UTC
I also note that the assignment routine in your 'for ($chn=0;$chn<16384;$chn++)' loop is not (or at least not obviously) definitive: '$energy_before[$chn]' could end up empty for various values of '$chn'. E.g., if $i was greater than $calib_points (I don't know if there's anything in your surrounding code that would prevent that.) This would cause line 1148 (at the very least) to throw an 'undefined' warning. -- Human history becomes more and more a race between education and catastrophe. -- HG Wells	[reply]
Re^2: Spurious `undefined' values and conversion issues by sf_ashley (Sexton) on May 30, 2008 at 18:45 UTC
A further thank you for your comment, oko1. $i should always be in range, as it is defined before the loop and can only be incremented if it is in an 'if' loop where $i < $calib_points. When $i == $calib_points, it will only go into the first loop. The reason for this is quite nice as it basically infers a particular set of interpolated data up to the last calibration point and beyond. Obviously, this isn't a great extrapolation, however, as this aforementioned calibration point is conveniently the last point on the spectrum, if does just the trick. Also, printing the values of $energy_before[] to a file showed that there weren't any non NULL events. Sadly, it was down to $j later on in the code. However, my sincere thanks for going through the code and for your comments. Steve	[reply]
Re: Spurious `undefined' values and conversion issues by apl (Monsignor) on May 30, 2008 at 01:24 UTC
You should fix the undefined and uninitialized errors long before you worry about anything else. (GIGO) If you add comments as to which of the lines in your excerpt are the offending ones (1148, 1153, 1158, etc.) we can better help you in resolving them.	[reply]
Re^2: Spurious `undefined' values and conversion issues by oko1 (Deacon) on May 30, 2008 at 01:31 UTC
He has, actually. They're not quite as visible as they might be - I was going to post the same thing as you did - but they're there, in-lined. -- Human history becomes more and more a race between education and catastrophe. -- HG Wells	[reply]
Re: Spurious `undefined' values and conversion issues by toolic (Bishop) on May 30, 2008 at 14:18 UTC
As an aside note, you could make some of your code look more Perl-ish. For example, instead of C-style "for" loops, you could take advantage of Perl's Range Operator (see perlop) by changing this: `$j=0; for ($j=0;$j<=16381;$j++){ if (defined($counts_rebin[$j])){ next; } else{ $counts_rebin[$j]=0; } }` [download] into this: `for my $j (0..16381) { $counts_rebin[$j] = 0 unless defined $counts_rebin[$j]; }` [download] You could even take things a step further and employ map to initialize the `@energy_rebin` array by changing this: `for ($i=0; $i<16834; $i++){ $energy_rebin[$i] = ($rebin_width * $i); }` [download] into this: `my @energy_rebin = map { $_ * $rebin_width } (0..16833);` [download]	[reply] [d/l] [select]
Re^2: Spurious `undefined' values and conversion issues by sf_ashley (Sexton) on May 30, 2008 at 18:54 UTC
As I am still rather new to programming with perl, I still think in terms of fortran, which I have had a little experience with. It is rather overwhelming, in terms of all the commands available, as to what one can do and it doesn't help when I still think very much in a limited sense to the basic commands and building up from there. However, the more I see and play with it, the more it will seep in, and your suggestion regarding map is very interesting. I'll have to read around that. I suppose, all in all, that at the monent, my handicap in perl golf is a little on the high side! Many thanks for your comment Steve	[reply]