in reply to Re^2: first stumbling steps in PDL
in thread first stumbling steps in PDL

Is there an advantage of using tricpy over mtri? At least mtri may be called without argument when an upper triangle is requested, while with tricpy it is always required.

Greetings,
-jo

$gryYup$d0ylprbpriprrYpkJl2xyl~rzg??P~5lp2hyl0p$

Replies are listed 'Best First'.
Re^4: first stumbling steps in PDL
by etj (Priest) on Feb 03, 2024 at 03:54 UTC
    I'd say use whatever you find works! tricpy should clearly allow a default value of 0. Feel like making a PR? You'd have to make a PMCode function :-)

      Opened an PR.

      PS:
      The second upvote for this node promoted me to Deacon.
      Thank you all!

      Greetings,
      -jo

      $gryYup$d0ylprbpriprrYpkJl2xyl~rzg??P~5lp2hyl0p$

        The second upvote for this node promoted me to Deacon. Thank you all!

        Well done 🐻!

        I see from Voting/Experience System that your next level is 3000 XP Curate, which for hysterical raisins ensures you'll deservedly appear on Saints in our Book ... making the 3000 XP target the most alluring to me, as indicated at Let's make BBQ a Saint!

        jo37, I've always enjoyed your posts and look forward to you finally scaling the 3000 XP plateau to deservedly join BBQ and other historical figures of the monastery.

        👁️🍾👍🦟
        jo37's excellent contribution has now been merged, and released with PDL::LinearAlgebra 0.38.
        Thank you for the PR! And congratulations on your ascension ;-)

      Now I have two different solutions to cope with a missing argument to tricpy. Both meet the target, as all these statements are valid and produce the same result:

      tricpy($m, 0, $c); $c = $m->tricpy(0); $c = $m->tricpy
      Though I like the first solution more, it raises a documentation issue.

      First:

      pp_def( 'tricpy', Pars => 'A(m,n);[o] C(m,n)', OtherPars => 'int uplo', OtherParsDefaults => {uplo => 0}, ArgOrder => [qw(A uplo C)], Code => ' ... ', Doc => <<EOT =for ref Copy triangular part to another matrix. If uplo == 0 copy upper triang +ular part. =cut EOT

      Second:

      pp_def( 'tricpy', Pars => 'A(m,n);uplo();[o] C(m,n)', PMCode => 'sub PDL::tricpy { return PDL::_tricpy_int(@_) if @_ == 3; my ($A, $uplo) = @_; PDL::_tricpy_int($A, $uplo // 0, my $C = PDL->null); $C; }', Code => ' ... ', Doc => <<EOT =for ref Copy triangular part to another matrix. If uplo == 0 copy upper triang +ular part. =cut EOT );

      The generated POD for the first version is misleading, as the signature doesn't conform to the actual argument order:

      tricpy Signature: (A(m,n);[o] C(m,n); int uplo) Copy triangular part to another matrix. If uplo == 0 copy upper triangular part. tricpy does not process bad values. It will set the bad-value flag + of all output ndarrays if the flag is set for any of the input ndarra +ys.

      Any suggestions?

      Greetings,
      -jo

      $gryYup$d0ylprbpriprrYpkJl2xyl~rzg??P~5lp2hyl0p$
        I like that you explored both options! You've joined quite a small select band of people who've used PP to make/modify functions in PDL. I do think that's a pity, since with the examples and hopefully the docs these days, more people should give it a go.

        OtherPars

        I thought of the OtherPars option, and was inclining away from it since leaving the flexibility of broadcasting over uplo seemed a bit more powerful. However, the clarity gained from having the calling code say what kind of tricpy is being done, and that not changing per broadcasted item seems more important on balance. To solve the documentation problem, use your example code in a usage section, like in the Ops ones:
        =for usage tricpy($m, 0, $c); # all params $c = $m->tricpy(0); # explicitly supply uplo $c = $m->tricpy; # uplo defaults to 0
        There's another outcome from using OtherPars: the if branches should each contain a broadcastloop for efficiency, since obviously the OtherPars won't change during broadcasting.

        Please do add tests for the new functionality.

        PMCode

        Minor nits for another time (please run with the OtherPars route): the initial shortcut should use  >= 3 in case the user supplied too many arguments and the XS can provide a good error. It looks like you dropped the int from the type, which is probably to be avoided.

        Complex data

        One of the legacy things that, at least for now, PDL::LinearAlgebra has to deal with is the complications of dealing with complex-number data in one of two ways, with the pp_defc function that does some horrible (albeit successful) mangling. I was just wondering whether that would require modifying here. It doesn't, since tricpy is handled in its own way, but until just now (including in the still-latest released version), the "real" version (which gets called for native-complex data) didn't handle native-complex types, and would have been silently coerced to real. I've now fixed that bug.

        The only consequence for this change is simply that in Complex/complex.pd there is a ctricpy which will need the same changes making to it as for the real version.

      Feel like making a PR?

      Sounds like an interesting challenge. I'll give it a try.

      Greetings,
      -jo

      $gryYup$d0ylprbpriprrYpkJl2xyl~rzg??P~5lp2hyl0p$