Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
fantasma13 opened this issue Aug 4, 2022 · 18 comments
Open

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

fantasma13 opened this issue Aug 4, 2022 · 18 comments

Comments

@fantasma13
Copy link
Contributor

fantasma13 commented Aug 4, 2022

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 mohawk2 changed the title NiceSlice test fails in 2.080 NiceSlice code with dubious "=over" fails in 2.080 Aug 4, 2022
@mohawk2
Copy link
Member

mohawk2 commented Aug 4, 2022

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
Copy link
Member

mohawk2 commented Aug 4, 2022

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
Copy link
Contributor Author

fantasma13 commented Aug 5, 2022

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
Copy link
Contributor Author

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
Copy link
Member

mohawk2 commented Aug 5, 2022

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

@fantasma13
Copy link
Contributor Author

fantasma13 commented Aug 22, 2022

#!/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
Copy link
Contributor Author

Text::Balanced Version 2.06

@fantasma13
Copy link
Contributor Author

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
Copy link
Member

mohawk2 commented Aug 22, 2022

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
Copy link
Contributor Author

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
Copy link
Member

mohawk2 commented Aug 26, 2022

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
Copy link
Contributor Author

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
Copy link
Member

zmughal commented Dec 24, 2022

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
Copy link
Member

zmughal commented Dec 25, 2022

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
Copy link
Member

zmughal commented Dec 26, 2022

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
Copy link
Member

zmughal commented Dec 26, 2022

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
Copy link
Member

zmughal commented Dec 26, 2022

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
Copy link
Member

zmughal commented Dec 27, 2022

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.

zmughal added a commit to PDLPorters/PDL-Graphics-Prima that referenced this issue Dec 30, 2022
  env PDL_NICESLICE_ENGINE='Filter::Simple' ...
  env PDL_NICESLICE_ENGINE='Filter::Util::Call' ...

Fixing this properly will depend on <PDLPorters/pdl#406>.

Adding this fix for now so that other fixes can move forward.
zmughal added a commit to PDLPorters/PDL-Graphics-Prima that referenced this issue Dec 30, 2022
  env PDL_NICESLICE_ENGINE='Filter::Simple' ...
  env PDL_NICESLICE_ENGINE='Filter::Util::Call' ...

Fixing this properly will depend on <PDLPorters/pdl#406>.

Adding this fix for now so that other fixes can move forward.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants