Open moocow-the-bovine opened 2 years ago
Thank you for your extremely thorough attention to this fascinating, subtle problem!
For the high-level PDL::CCS::Nd wrappers, I think it's always the case that
NNzOut==NnzIn
, so it doesn't make sense there. I can imagine cases with structured data where it does make sense though, and it could be wasteful (especially for ufuncs) to forceNNzOut==NnzIn
there, so I'm inclined to leave those dimensions distinct.
My recommendations are based on what I've learned from investigating this, and also from my recent experience of using a lot of RedoDimsCode
in PDL::LinearAlgebra so I could just pass in null
instead of the agonising nonsense with zeroes
. They are (I removed the ones you've indicated above since you obviously know your library better than I!):
PDL_MAYBE_SIZE
from https://github.com/PDLPorters/pdl-linearalgebra/blob/f789c4100d04ba9d1b50f8c18249bdef29338496/Real/real.pd#L63-L75, for example at https://github.com/PDLPorters/pdl-linearalgebra/blob/f789c4100d04ba9d1b50f8c18249bdef29338496/Real/real.pd#L204-L208 - this only sets the dimsize if the passed ndarray is a null
null
as the output params but would still allow you to not do so (though, srsly, why)Code
with $PDLSTATESETBAD(ndarrayname)
, outside an inserted broadcastloop
- see https://github.com/PDLPorters/pdl/commit/76f353eef645728f0df59172fcb92f12c6b5344a for examplesCopyBadStatusCode
just call the PDL.propagate_badflag
with the set values (i.e. PDL->propagate_badflag(ndarrayname, !!(ndarrayname->state & PDL_BADVAL));
), i.e. to do the same as the new API doesmissing
Par be the last value of your valsIn
, use RedoDimsCode
to give that a dim of NnzInPlus1
, and in loop (NnzInPlus1) %{ %}
use an if (NnzInPlus1 == $SIZE(NnzInPlus1)-1) break;
at the top to not accidentally process the missing valueI believe with those points, you could heavily reduce the PMCode
(though not eliminate thanks to the slicing requirement).
broadcastloop
and $PDLSETSTATE*(ndarrayname)
in Code
appear not to work with my (probably outdated) PDL-2.019 (-> yes, I should upgrade, but am not going to do that right now)RedoDimsCode
(in a way which is compatible with older PDLs), and minimizing/eliminating PMCode
are all out of my (TUIT) price range at the moment... leaving this issue open for now, although I may copy+paste your suggestions (for which many thanks, in particular for the linked examples!) into separate issues later.
Switching the minimum PDL 2.081, as discussed on #13, will allow using indx
where appropriate, which will solve the type mismatch issue.
see https://github.com/moocow-the-bovine/PDL-CCS/pull/5#issuecomment-1100614446
out_type => 'indx'
rather thanout_type => 'int+'
ccs_accum_nbad
should ensure$missing->type == $nzvalsIn->type