PDLPorters / pdl

Scientific computing with Perl
http://pdl.perl.org
Other
91 stars 46 forks source link

NiceSlice code with dubious "=over" fails in 2.080 #406

Open fantasma13 opened 2 years ago

fantasma13 commented 2 years ago

HI Ed, make test fails with NiceSlice errors, so do a lot of programs. Both latest git as from CPAN. 2.079 still works. Is this maybe related to Text-Balanced? Installed version is 2.06. Debian linux.

Here's a quite minimal example. I think it has to do with ( ) or {} in comments.

PDL 2.080: perl -Mblib -e "use test_ns; "

syntax error at /home/ingo/perl/test_ns.pm line 24, near "$iii("
Compilation failed in require at -e line 1.
BEGIN failed--compilation aborted at -e line 1.

PDL 2.079: perl -Mblib -e "use test_ns; "

syntax error at /home/ingo/perl/test_ns.pm line 24, near "$iii("
Compilation failed in require at -e line 1.
BEGIN failed--compilation aborted at -e line 1.

test_ns.pm:

#!/usr/bin/perl  

package test_ns;
use PDL;
use PDL::NiceSlice;

=over
sub fmod {
        my $x=shift;
        my $y=shift; 
        my $n=floor $x/$y; 
        return $x-$n*$y;
}

sub amp {
        my $i=shift; 
        return (sqrt($i((0),)**2+$i((1),)**2));
}
=cut

sub phase {
        my $iii=shift;
        return (atan2($iii((1),),$iii((0),)));
}

# { 
# #} sMDH;                                        // total length: 32 * 32 Bit (128 Byte)            128
#

1;
mohawk2 commented 2 years ago

Thank you for the report. As you can see, I adjusted the title because this does not report a "NiceSlice test" failing (it's not in the NiceSlice test suite).

I also had to edit the text of your report to put in the appropriate ``` markers to make the code and output look like I believe you intended to. Please, before you add comments and/or report problems, use the "Preview" tab at the top of the text entry box, to make sure it looks as you intend. I always do.

I am confused by what you wrote: you say "make test fails with NiceSlice errors". I have to assume you mean in your code, because PDL's "make test" passes just fine. Please be clear with that sort of thing :-)

Most of all, I am very confused in that you say "2.079 still works", but then PDL 2.079: perl -Mblib -e "use test_ns; " with an error. Does your code work with 2.079?

Also, please try adjusting your code to have a blank line after =over. If that then works, please adjust your report. In any case, it's most helpful for a bug report to show as little code as possible that still reproduces the error.

mohawk2 commented 2 years ago

Another thing to try is to put a blank line before =cut and see if that then works. https://perldoc.perl.org/perlpod#=cut says:

=cut

To end a Pod block, use a blank line, then a line beginning with "=cut", and a blank line after it. This lets Perl (and the Pod formatter) know that this is where Perl code is resuming. (The blank line before the "=cut" is not technically necessary, but many older Pod processors require it.)

If that in fact works, then Text::Balanced (and PDL::NiceSlice) are working correctly according to the POD spec, even if they need adjusting for real-world POD parsing by Perl itself.

fantasma13 commented 2 years ago

HI Ed, I did this in a kind of a hurry. My student reported an error with our code and I hoped to quickly fix it. Then I ran make test on the latest git release, which failed nicesclice.t. I realised later on that Text::Balanced was still on 2.04, the error in the test suite disappeared after going to 2.06. The real problem remained, however.

Adding blank lines around =over and =cut does not change anything.

In 2.079, Text::Balanced reports this (I added a print statement at the end of the script to see how far it gets):

perl -Mblib ~/perl/test_ns.pm

Prototype mismatch: sub Text::Balanced::_match_variable: none vs ($$) at /data/ingo/PDL-2.079/blib/lib/PDL/NiceSlice.pm line 603. Prototype mismatch: sub Text::Balanced::_match_codeblock: none vs ($$$$$$$) at /data/ingo/PDL-2.079/blib/lib/PDL/NiceSlice.pm line 605. Prototype mismatch: sub Text::Balanced::_match_quotelike: none vs ($$$$) at /data/ingo/PDL-2.079/blib/lib/PDL/NiceSlice.pm line 607. hello world!

I am not sure if these are errors or warnings.

fantasma13 commented 2 years ago

Put these lines towards the end and run it under 2.079 and it gives the desired result of phase();

my $n=pdl(1,1); print "hello world! ",phase($n),"\n";

mohawk2 commented 2 years ago

Please post some code (using ``` markers) that you are saying fails under 2.080, and confirm what version of Text::Balanced you have.

fantasma13 commented 2 years ago
#!/usr/bin/perl  

package test_ns;
use PDL;
use PDL::NiceSlice;

=over 

sub fmod {
    my $x=shift;
    my $y=shift; 
    my $n=floor $x/$y; 
    return $x-$n*$y;
}

sub amp {
    my $i=shift; 
    return (sqrt($i((0),)**2+$i((1),)**2));
}

=cut

sub phase {
    my $iii=shift; 
    return (atan2($iii((1),),$iii((0),)));
}

# { 
# #} sMDH;                                        // total length: 32 * 32 Bit (128 Byte)            128
#

my $n=pdl(1,1);
print "hello world! ",phase($n),"\n";

1;
fantasma13 commented 2 years ago

Text::Balanced Version 2.06

fantasma13 commented 2 years ago

If you don't like my example, here's another problem, PDL::Graphics::Prima.

PDL 2.079 and Text::Balanced 2.06 prints these warnings.

(base) heiner:[pdl-code]$ perl -e 'use PDL::Graphics::Prima;' Prototype mismatch: sub Text::Balanced::_match_variable: none vs ($$) at /usr/local/lib/x86_64-linux-gnu/perl/5.34.0/PDL/NiceSlice.pm line 603. Prototype mismatch: sub Text::Balanced::_match_codeblock: none vs ($$$$$$$) at /usr/local/lib/x86_64-linux-gnu/perl/5.34.0/PDL/NiceSlice.pm line 605. Prototype mismatch: sub Text::Balanced::_match_quotelike: none vs ($$$$) at /usr/local/lib/x86_64-linux-gnu/perl/5.34.0/PDL/NiceSlice.pm line 607.

PDL 2.080 gives a syntax error.

(base) heiner:[pdl-code]$ perl -Mblib -e 'use PDL::Graphics::Prima;' syntax error at /usr/local/share/perl/5.34.0/PDL/Graphics/Prima/DataSet.pm line 1369, near "$data(" Global symbol "$log_spaces" requires explicit package name (did you forget to declare "my $log_spaces"?) at /usr/local/share/perl/5.34.0/PDL/Graphics/Prima/DataSet.pm line 1370. BEGIN not safe after errors--compilation aborted at /usr/local/share/perl/5.34.0/PDL/Graphics/Prima/DataSet.pm line 1387. Compilation failed in require at /usr/local/share/perl/5.34.0/PDL/Graphics/Prima.pm line 39. BEGIN failed--compilation aborted at /usr/local/share/perl/5.34.0/PDL/Graphics/Prima.pm line 39. Compilation failed in require at -e line 1. BEGIN failed--compilation aborted at -e line 1.

mohawk2 commented 2 years ago

Your first message needs three backquotes at the start and finish of the block to be quoted, not one. That is why it renders wrongly. If you use the "Preview" function above the text box, you can see this before you post. After posting, it's possible to edit the text using the "..." menu top right of the message, selecting "Edit".

More substantively, that code has =over but lacks an =back. Is that what you intended? I agree this shows a problem with Text::Balanced.

fantasma13 commented 2 years ago

Adding =back seems to mask the issue. This was just a minimal example demonstrating the problem, it is not unique, I think. Issues are with () {} ... in comments or pod of some sort, I think.

mohawk2 commented 2 years ago

PDL::Graphics::Prima triggers warnings in 2.079 with T:B 2.06 because it uses PDL::NiceSlice; I am pretty sure you'd have seen the same if you'd done use PDL::NiceSlice.

The difference between P:G:P behaviour on the two PDLs is that 2.080 removed the vast amounts (about 500 lines) of overriding code. It looks like P:G:P demonstrates further bugs in T:B or Filter::Simple, which I will look into along with yours.

In terms of the POD in your example code, Filter::Simple (https://metacpan.org/dist/Filter-Simple/source/lib/Filter/Simple.pm#L37-40) only considers =head[1-4]|item|pod|for|begin as opening a POD section. It's quite possible that should just be anything starting with =, since it seems Perl treats it that way (at least on my reading of the tokeniser at https://github.com/Perl/perl5/blob/blead/toke.c#L9044). It does look like there's some mismatch between T:B, Filter::Simple, and PDL::NiceSlice that these two examples show.

By the way, referring to "mask"ing the issue doesn't seem accurate: either your code triggers a NiceSlice bug or it doesn't.

fantasma13 commented 2 years ago

OK, Ed, thank you. Here's one that triggers it without comments.


#!/usr/bin/perl  

package MRI::Utils;

use PDL::NiceSlice;

our $VERSION=1.000_000;
use strict;
sub add_phase{
    my $x=shift;
    my $y=shift;
    return atan2(sin($x+$y),cos($x+$y));
}

sub fmod {
    my $x=shift;
    my $y=shift; 
    my $n=floor $x/$y; 
    return ($x-$n*$y);
}

sub amp {
    my $i=shift; 
    return (sqrt($i((0),)**2+$i((1),)**2));
}

sub phase {
    my $iii=shift; 
    return (atan2($iii((1),),$iii((0),)));
}

sub phase_diff {
    my $r=shift; # first complex number
    my $i=shift;
    my $pr=shift; # 2nd complex number
    my $pi=shift;
        my $a=sqrt ($r**2+$i**2); # Amplitudes
        my $pa=sqrt ($pr**2+$pi**2);
        my $ca=$r/($a+($a==0)); # sin/cos a
        my $sa=$i/($a+($a==0));
        my $cb=$pr/($pa+($pa==0)); # sin/cos b
        my $sb=$pi/($pa+($pa==0));
        my $cab=$ca*$cb+$sa*$sb;
        my $sab=$sa*$cb-$ca*$sb;
        return ($a*($cab), $a*($sab)); 
}

sub unmask {
    my $mask=shift;
    my $str='';
    for my $i (0..31) {
        $str.="$i " if ($mask & 2**$i);
    }
    return $str."\n";
}

1;

__END__
zmughal commented 1 year ago

The following regression came about with code taken from PDL::Graphics::Prima:

commit 0c6f5b92fcdff03a52a7a3ab08cfa5e16bd13099 (HEAD -> gha-ci, origin/gha-ci)
Date:   Fri Dec 23 19:18:14 2022 -0500

    drop! wip: "fix" regression in Filter::Simple relative to Filter::Util::Call

      env PDL_NICESLICE_ENGINE='Filter::Simple' ...
      env PDL_NICESLICE_ENGINE='Filter::Util::Call' ...

diff --git a/lib/PDL/Graphics/Prima/DataSet.pm b/lib/PDL/Graphics/Prima/DataSet.pm
index 39fb207..01f3119 100644
--- a/lib/PDL/Graphics/Prima/DataSet.pm
+++ b/lib/PDL/Graphics/Prima/DataSet.pm
@@ -1426,7 +1426,7 @@ sub guess_scaling_for {
        my $lin_space = $lin_spaces->average;
        my $lin_score = (($lin_spaces - $lin_space)**2)->sum / $lin_spaces->nelem;

-       my $log_spaces = $data(1:-1) / $data(0:-2);
+       my $log_spaces = $data->slice('1:-1') / $data(0:-2);
        my $log_space = $log_spaces->average;
        my $log_score = (($log_spaces - $log_space)**2)->sum / $log_spaces->nelem;

I have extracted the two lines which fail to filter together from this:

#!/usr/bin/env perl

use PDL;
use PDL::NiceSlice;
    my ($data, $lin_spaces);
    my $lin_score = (($lin_spaces - $lin_space)**2)->sum / $lin_spaces->nelem;
    my $log_spaces = $data(1:-1) / $data(0:-2);
no PDL::NiceSlice;
$ for E in Filter::Util::Call Filter::Simple; do env PDL_NICESLICE_ENGINE=$E bash -c 'echo $PDL_NICESLICE_ENGINE && perl -c ns-reg.pl' ; done
Filter::Util::Call
ns-reg.pl syntax OK
Filter::Simple
syntax error at ns-reg.pl line 7, near "$data("
BEGIN not safe after errors--compilation aborted at ns-reg.pl line 8.
$ for i in PDL Text::Balanced; do echo "$i $(mversion $i)"; done
PDL 2.081
Text::Balanced 2.06

Given that the / operator is significant for both lines, I'm guessing this is being parsed as a regex /RE/. Changing ->sum to ->sum() fixes it.

zmughal commented 1 year ago

A smaller example that breaks:

#!/usr/bin/env perl

use PDL;
use PDL::NiceSlice;
    my ($data, $lin_spaces);
    my $lin_score = ($lin_spaces)->sum / $lin_spaces->nelem;
    my $log_spaces = $data(1:-1) / $data(0:-2);
no PDL::NiceSlice;

The () around $lin_spaces and the two / are all necessary for the filter to fail with Filter::Simple.

zmughal commented 1 year ago

By the way, to debug the filtered output of a given engine, you can use Filter::tee.

Add

use Filter::tee ">$0-$ENV{PDL_NICESLICE_ENGINE}";

before the lines you want to see filtered then run

for E in Filter::Util::Call Filter::Simple; do env PDL_NICESLICE_ENGINE=$E perl -c file-to-filter.pl; done
zmughal commented 1 year ago

From IRC:

04:02:01 [@sivoais] perlbot: eval: use Text::Balanced; $Text::Balanced::VERSION
04:02:03 [ perlbot] sivoais: 2.06 🎄
04:04:15 [@sivoais] perlbot: eval: use Text::Balanced ':ALL'; $tt = 'my $X = ($foo)->a / $bar; my $Y = $bar / 1;'; pos($tt) = 0 ; join ", ", map "qq[$_]",
                    extract_multiple($tt)
04:04:16 [ perlbot] sivoais: qq[my ], qq[$X], qq[ = (], qq[$foo], qq[)->a ], qq[/ $bar; my $Y = $bar /], qq[ 1;] 🎄
zmughal commented 1 year ago

Another way to debug source filters is to use Filter::ExtractSource:

diff <( PDL_NICESLICE_ENGINE=Filter::Util::Call perl -MFilter::ExtractSource -c file-to-filter.pl ) \
     <( PDL_NICESLICE_ENGINE=Filter::Simple     perl -MFilter::ExtractSource -c file-to-filter.pl )
zmughal commented 1 year ago

OK, Ed, thank you. Here's one that triggers it without comments.

Using this, I created the following test case that fails:

#!/usr/bin/perl  

use PDL::NiceSlice;

sub foo {
    my ($x,$y) = @_;
    $x / $y; 
}

sub bar {
    my ($r,$i) = @_;
    $r/($i);
}

1;

with a diff between the P:NS engines of:

12c12
<   $r/($i);
---
>   $r/->slice($i);

This occurs even when the statements are in the same sub.