Menu

#411 relax reshape() constraints

feature_request
closed
nobody
None
5
2022-04-14
2015-11-19
No

To avoid a bug in the reshape() routine, it no longer will operate on a PDL with data flow active. This avoids an interaction between reshape() and its dimension mangling and mv() which can re-order dimensions in such a way that the reshape() implementation ends up indexing out of the piddle data. This was motivated by the fact that reshape() is mainly for turning a contiguous block of data into and equivalent multidimensional piddle and so applying it to a piddle with scrambled axis orders doesn't really make sense.

However, many uses of reshape() are essentially shorthand for a sequence of splitdim() operations and it would be nice if this were allowed. I'm opening this ticket as a reminder to address this following the coming PDL-2.015 release. As always, feedback and suggestions on bug fixes are welcome.

Discussion

  • Craig DeForest

    Craig DeForest - 2015-11-19

    The original bug happens because of the interaction between reshape()'s non-canonical / legacy inplace operation, and the way mv() establishes a trans to relate dimensions. mv() establishes a trans() structure that relates a certain dimension (the n1 field in the trans) in the parent to the corresponding other dimension (the n2 field in the trans). The dimlists implicitly have the same length. If reshape() is carried out on either of the PDLs in the mv() relationship, then it is possible to shorten the dimlist of one so that it the mv() relationship is no longer valid.

    The real problem isn't with using reshape() as a shorthand for a sequence of splitdim operations, it's with using reshape to modify a PDL that is itself an endpoint of a particular trans.

    It's possible to do that with sever() -- e.g.

    $a = pdl([0,1],[2,3]);
    $b = $a->mv(-1,0)->sever->reshape(4);

    Another convenient way to implement reshape-as-splitdim-sequence cleanly would be to write a new method (maybe called newshape?) that produces a new PDL and links it to the parent with a new kind of trans structure, rather than (as reshape does) modifying the parent in-place.

     
  • Craig DeForest

    Craig DeForest - 2015-11-19

    To be even more clear, the child of an mv() operation doesn't actually have its own dimlist or data pointer: the trans code mocks up the dimlist pointer on-the-fly based on the dimlist of the parent. So modifying the mocked-up dimlist in-place with reshape/setdims is a very bad idea (which is why it corrupts the parent PDL's data and/or causes segfaults).

     
    • Chris Marshall

      Chris Marshall - 2015-11-19

      Definitely not something to get into for PDL-2.015! Thanks for the
      explanations.

      On Thu, Nov 19, 2015 at 12:18 PM, Craig DeForest zowie@users.sf.net wrote:

      To be even more clear, the child of an mv() operation doesn't actually
      have its own dimlist or data pointer: the trans code mocks up the dimlist
      pointer on-the-fly based on the dimlist of the parent. So modifying the
      mocked-up dimlist in-place with reshape/setdims is a very bad idea (which
      is why it corrupts the parent PDL's data and/or causes segfaults).


       
  • mohawk

    mohawk - 2018-04-09

    This matter will now be tracked at https://github.com/PDLPorters/pdl/issues/217

     
  • mohawk

    mohawk - 2022-04-14
    • status: open --> closed
     

Log in to post a comment.