Implement rotation methods

[Shir0kamii]

Hi,

Since the grid library has been useful for a personnal project, I wanted to contribute and tried to tackle #16.

I wondered if I should add unit tests but I think the doc-tests are enough, right ?

I also took the liberty of implementing 180° rotation, to have full rotation capabilities.

I hope this helps!

[becheran]

@Shir0kamii thanks for the PR. Just had a brief look and it seems to work.

Though, it seems a bit inefficient to copy the full vec. I am not 100% sure if it is possible, but it would be great to avoid the aloc and do the operation within the existing array by "resorting" the elements. That would decrease memory consumption and most likely be a lot faster. Though I am not sure if something for rotate exists with O(n). I don't know the algorithm yet, but I guess it must be possible.

[Shir0kamii]

I tried to follow how the Grid::transpose method is implemented. To do what you suggest, the signature would need to be either rotate(&mut self) or rotate(self) -> Self, which would create discrepancy with the transpose.

Still, your suggestion piqued my curiosity, and I started searching how in-place rotation could be achieved. I couldn't find anything by toying around on paper, but thankfully, the internet is here to help us.

According to my research, rotating a grid in-place involves transposing in-place. While in-place transposing of a square grid is trivial, it seems to be impossible for a rectangular grid without using auxiliary storage. I found a C++ implementation that does it with a O(n) memory footprint. According to Wikipedia, it is possible with O(log n) memory, at the cost of O(n log n) processing, but I couldn't find the mentionned algorithm.

I'm not sure allocating a Vec<bool> the same size of the grid then doing the in-place transpose + reversal of rows/cols would be faster than the current implementation, but I can try it if you want.

On the other hand, it would make possible to drop the Clone bound, which would make it faster for non-Copy types. But I suspect most types used in grids are Copy, so I don't know how good it is.

[Shir0kamii]

I tried implementing the methods in-place, following the previously linked C++ implementation, and benchmarked them against the current implementation.

Surprisingly, they are more than 2 times slower. This difference is mostly explained by the required in-place transpose, which dominate the computation time.

I pushed this implementation on my fork under the branch 'rotate-inplace', if you want to take a look.

[becheran]

Thanks a lot for taking the time!

I was a bit optimistic when I thought it would be easy to do it in O(n). You are right. Doing it for square grids is fairly easy, but the rectangular part is quite hard.

I would suggest that I merge this PR and create a follow up ticket to investigate if performance can be improved. Now I remember that when I pulled the transpose method in the repo I already stumpled upon the complexity of doing the transpose "in-place" after I merged the PR. Wanted to investigate further but forgot.

So maybe if the transpose method can be improved, the rotate part might also benefit from it. But then it would make sense to change the signature of both rotate and transpose to '(&mut self)' I guess.

[Shir0kamii]

We can certainly proceed as you said.