xform bvalflags not being propagated in core, but by PP code

[vitstradal]

Maybe I do not undersand fully, but this script produce different outputs on 2.82 and 2.95:

use warnings;                                                                   
use feature 'say';                                                              
use PDL;                                                                        
use Inline Pdlpp => 'DATA';                                                     
                                                                                
my $bad_with_bad = pdl([double->badvalue]);                                     
$bad_with_bad->badflag(1);                                                      
$bad_with_bad->bad_to_666();                                                    
say "very bad piddle=$bad_with_bad";                                            
                                                                                
my $bad_with_good = pdl([10]);                                                  
$bad_with_good->badflag(1);                                                     
$bad_with_good->bad_to_666();                                                   
say "bad piddle=$bad_with_good";                                                
                                                                                
my $good = pdl([10]);                                                           
$good->bad_to_666();                                                            
say "very good piddle=$good";                                                   
                                                                                
__DATA__                                                                        
__Pdlpp__                                                                       
                                                                                
pp_add_exported(qw/bad_to_666/);                                                
pp_def( 'bad_to_666',                                                           
        Pars => 'vec(n)',                                                       
        HandleBad => 1,                                                         
        Code => '$vec(n=>0) = 911;',                                            
        BadCode => q@                                                           
           if ($ISGOOD(vec(n=>0))) {                                            
               $vec(n=>0) = 69;                                                 
           } else {                                                             
               $vec(n=>0) = 666;                                             
           }                                                                    
        @,                                                                      
);

2.95 (BadCode is ignored and only Code is used):

very bad piddle=[911]                                                           
bad piddle=[911]                                                                
very good piddle=[911]

2.85 (as I expected):

very bad piddle=[666]                                                           
bad piddle=[69]                                                                 
very good piddle=[911]                                                          

[mohawk2]

Using BadCode is not now the recommended way (use PDL_IF_BAD(t,f) in Code), but it's not supposed to be ignored! I'll check.

(They normally get called "ndarrays" these days, by the way, not "piddles" ;-)

[mohawk2]

Good news! The BadCode isn't being ignored! But the xform's bvalflag isn't being set correctly in some cases. To see this in action:

use warnings;
use feature 'say';
use PDL::LiteF; # so smaller build during bisect
use Inline Pdlpp => 'DATA';

my $bad_with_bad = pdl('[BAD]');
$bad_with_bad->bad_to_666();
say "very bad ndarray=$bad_with_bad";
my $bad_with_good = pdl([10]);
$bad_with_good->badflag(1);
$bad_with_good->bad_to_666();
say "bad ndarray=$bad_with_good";
my $good = pdl([10]);
$good->bad_to_666();
say "very good ndarray=$good";
# very bad ndarray=[911]
# bad ndarray=[911]
# very good ndarray=[911]

$bad_with_bad = pdl('[BAD]');
$bad_with_bad->bad_to_666_io();
say "[io] very bad ndarray=$bad_with_bad";
$bad_with_good = pdl([10]);
$bad_with_good->badflag(1);
$bad_with_good->bad_to_666_io();
say "[io] bad ndarray=$bad_with_good";
$good = pdl([10]);
$good->bad_to_666_io();
say "[io] very good ndarray=$good";
# [io] very bad ndarray=[911]
# [io] bad ndarray=[911]
# [io] very good ndarray=[911]

$bad_with_bad = pdl('[BAD]')->bad_to_666_o();
say "[o] very bad ndarray=$bad_with_bad";
$bad_with_good = pdl([10]);
$bad_with_good->badflag(1);
$bad_with_good = $bad_with_good->bad_to_666_o();
say "[o] bad ndarray=$bad_with_good";
$good = pdl([10])->bad_to_666_o();
say "[o] very good ndarray=$good";
# [o] very bad ndarray=[666]
# [o] bad ndarray=[69]
# [o] very good ndarray=[911]

PDL::bad_to_666_o(pdl('[BAD]'), $bad_with_bad = null);
say "2 [o] very bad ndarray=$bad_with_bad";
$bad_with_good = pdl([10]);
$bad_with_good->badflag(1);
PDL::bad_to_666_o($bad_with_good, my $bad_with_good2 = null);
say "2 [o] bad ndarray=$bad_with_good2";
PDL::bad_to_666_o(pdl([10]), $good = null);
say "2 [o] very good ndarray=$good";
# 2 [o] very bad ndarray=[666]
# 2 [o] bad ndarray=[69]
# 2 [o] very good ndarray=[911]

__DATA__
__Pdlpp__

pp_def('bad_to_666',
  Pars => 'vec(n=1)',
  HandleBad => 1,
  Code => '$vec(n=>0) = PDL_IF_BAD($ISGOOD(vec(n=>0)) ? 69 : 666,911);',
);
pp_def('bad_to_666_io',
  Pars => '[io] vec(n=1)',
  HandleBad => 1,
  Code => '$vec(n=>0) = PDL_IF_BAD($ISGOOD(vec(n=>0)) ? 69 : 666,911);',
);
pp_def('bad_to_666_o',
  Pars => 'vec(n=1); [o] out(n)',
  HandleBad => 1,
  Code => '$out(n=>0) = PDL_IF_BAD($ISGOOD(vec(n=>0)) ? 69 : 666,911);',
);

[mohawk2]

I'm not sure what your testing was; 2.085 is also broken. 2.077 is good.

[mohawk2]

Process to find:

git bisect start 
git bisect bad 2.085
git bisect good 2.077
# repeat with git bisect good|bad until found:
time perl Makefile.PL && time make core && rm -rf _Inline && perl -Mblib repro-script

Broken in (by the commit message, you can tell l got it wrong), which was dev-released in 2.082_01 then immediately released with 2.083: 78c66ea "no emit BADFLAGCACHE-setting/using if nothing to do".

[vitstradal]

Oh, very sorry for confusion. Version 2.82 is working (not 2.85).

[mohawk2]

@vitstradal Thank you very much for this report! I can guess at why your version-reporting was a little off; one reason might be the problem I had, with not finding any changes until I went through it again, doing the rm -rf _Inline step. That turned out to be extremely important, since it was PP that was wrong, so it needed recompiling the generated code each time to detect that git commit's actual behaviour for this.

The problem here was the PP code to get the bvalflag stopped getting emitted. That was wrong, since it doesn't just report a flag, but also has a side-effect in setting the xform's bvalflag, as mentioned above. It mostly still worked, since most xforms have [o] parameters, but e.g. indadd wouldn't have.

A fix for this is very easy, but I don't want to close this yet, since the whole badflag communicating needs fixing. The fact that PP code is needed to make it operate is a vestige of an earlier time, before I refactored all the common code being emitted into each XS function into the core, but unfortunately left the calls in PP code. Soon I intend to make more of the xform life-cycle be rearranged, with things like badflag communicating done by readdata, which is where that belongs.

[mohawk2]

@vitstradal Could you check you agree the above commit fixes this? However, please don't close the issue, as more is needed from me :-)

[vitstradal]

I confirm that all my reproducers are passing (=behave correctly) in master ( f64d98c059f6 ).

[vividsnow]

Hi,

related issue: encountered badflag propagation to input value while using pp-based PDL::Finance::TA. Debug log for ta_sma function call: https://0x0.st/8oox.txt

[mohawk2]

@vividsnow I believe that as of d279bb7, the input to bad-setting operations don't get their badflag set. Could you try latest master and see? (The test that ensure this uses setbadif, which is similar, so it really should now be fine)