Complex out

[wlmb]

No description provided.

[wlmb]

Tried to implement PDL::Complex outputs for PDL::Complex inputs, where (hopefully) it makes sense.

[coveralls]

Coverage remained the same at 60.163% when pulling 816204c on wlmb:complex-out into fff36d8 on PDLPorters:master.

[mohawk2]

Thanks! I've rebased it, since there were merge commits in there causing confusion. I also integrated this into the changes I've made (and released as 0.11) which enable native complex inputs.

Could I ask you to try rebasing to avoid merge commits, going forward? Also, it's a bit confusing using ok_should_reuse_plan on tests that have nothing to do with making or reusing the plans! Take a look at the updated "Files changed" on here to see the updated code :-) Finally, you had $arg->isa('PDL') or ..., which if the $arg is a PDL::Complex will always be true, so the rest of the expression was never tested, so I deleted it.

As you can see, I've now merged this and released it as 0.12. Could you push your Photonic commits that utilise this (don't forget to update the dependency in Makefile.PL)? I'm having a go at changing it to actually use native complex, but I don't want to clash with your changes :-)

[wlmb]

Sorry but I'm ignorant about 'rebasing', and don't understand the problem on 'merge commits going forward'. I'll try to read abour that. I agree about ok_should_reuse_plan, but I saw all tests were using it, even for those where I wouldn't expect them to be meaningful. I'll look at the final code. Thanks for pointing out the isa('PDL') mistake. I look forward to using the native complex type. I'll look then at how Photonic works with 0.012. Regards.

[mohawk2]

I know it's a lot to take on - it took me literally months to get even a basic understanding. I'll "start with why"; here is the commit history for this PR, before I rebased it (I used git reflog to find the relevant commit which still exists in my repo till it gets removed by "garbage-collection", since it's not referenced anymore):

$ git log --oneline --graph -20 5c29c44
* 5c29c44 Add PDL::Complex outputs to Changes
*   06bc5bd Merge branch 'master' of https://github.com/PDLPorters/pdl-fftw3 into complex-out-1
|\  
| * e41e602 avoid clash with C11 static_assert
| * 3f6d08c no need string-eval
| * 39fd8ca more accurate line-numbers
| * c4e7762 doc and generalise PDL::Complex test handling slightly
* |   f55fdeb Merge branch 'master' of https://github.com/PDLPorters/pdl-fftw3 into complex-out-1
|\ \  
| * | 18a4017 more accurate line-numbers
| * | d92da7e doc and generalise PDL::Complex test handling slightly
| |/  
* | 3c06636 Change logical operators due to precendence
* | 23709ba Return PDL::Complex types for some arguments.
|/  
* b7c8a69 (tag: 0.10, l/master) 0.10
* 5b31708 delete obsolete "provides" data
* 68132fd (tag: 0.09) 0.09

My assumption is that you were doing git pull, which by default does a fetch followed by a merge, which by default makes a merge-commit if it can't fast-forward, which led to those two commits with "Merge" in - possibly using an IDE. This is deeply confusing because you can see two commits of mine ("doc and generalise" and "more accurate line-numbers") actually occur twice each. This resembles the tangled mess visible in the pdl repo's history around 64-bit indexing, with a much deeper, more tangled mess of merge commits. This would cause problems for people trying to figure out what was changed when, and especially why.

Here is the history after I rebased it:

$ git log --oneline --graph -20 
* bba9e2e (HEAD -> master, o/master, o/HEAD) capture value in variable
* 7c190d7 (tag: 0.12) 0.12
* 816204c (l/complex-out, complex-out) Return PDL::Complex types for some arguments.
* fff36d8 (tag: 0.11) 0.11
* d99e2e4 forward and reverse native complex<->complex
* 6d73a92 better failure-reporting in test
* e46792d calculate output type in same func as dims
* 8d27d56 DRY in dims-checking
* 68cb8e9 DRY in arg-checking
* 959a5ef reduce redundancy for internal function-name
* 26ac555 use X not N for FFT rank description
* ced1da9 simplify ternary op
* 3693618 generalise %pp_def slightly
* e41e602 avoid clash with C11 static_assert
* 3f6d08c no need string-eval
* 39fd8ca more accurate line-numbers
* c4e7762 doc and generalise PDL::Complex test handling slightly

This article gives some insight into the issues: https://stackoverflow.com/questions/25430600/difference-between-git-pull-rebase-and-git-pull-ff-only . I don't have all the answers, and I encourage you to do your own research - e.g. you may prefer to read about this in another language than English. Obviously, I'm pleased to answer any questions!

One generality I will say is that the commits you did make (3 of them) really added up to just one (really good) change, and having them separated out wasn't the most useful thing. To fix that, what I'd have done (and in fact did do) was git rebase -i master, and use either squash or fixup to combine the commits with a single, helpful-to-future-readers, message.

Please do keep up the contributing, it's really helpful (and encouraging to me). Don't worry if this git thing takes some getting used to - I'd much rather have to do a bit of git untangling than not have your stuff at all :-)

[wlmb]

Wow. Thanks for the detailed explanation. It will be very helpful for the future, though I'm sure I'll make many errors (and maybe the same mistakes again and again, as I always forget faster than I learn with git). Regards.

[dkogan]

Just want to say thanks to yall for keeping this going.

[mohawk2]

Wow. Thanks for the detailed explanation. It will be very helpful for the future, though I'm sure I'll make many errors (and maybe the same mistakes again and again, as I always forget faster than I learn with git). Regards.

Experience is what lets me recognise mistakes when I make them again ;-) Don't worry, it will all be fine!

[mohawk2]

Just want to say thanks to yall for keeping this going.

@dkogan Thanks for creating this impressive piece of work! Any time you want to dip your toe in the water again... :-D