Refactor steerable pyramid

[billbrod]

Refactor SteerablePyramidFreq, similar to the PortillaSimoncelli refactor in #225. Like the PortillaSimoncelli code before that PR, the steerable pyramid code is more or less a straight port from the matlab (by way of pyrtools). It would be nice to refactor, clean it up, and speed it up.

Copying Edardo's comments from that. All these comments are in __init__:

Line 50, about the docstring for twidth: I don't love this compact name, something like transition_width would be more explicit

Line 61, about the explanation of tight frame: In the attributes section, it looks like you have some attribute missing: order, downsample, tight_frame, alpha, lutsize... some are listed as Parameters and some are not. if this is by choice because you do not want to replicate the docstrings of the Parameters of the__init__, you should remove is_complex and image_shape) because they are redundant.

Line 118, about definition of self.Xcosn: what are this for? Xcosn and alpha? it would be nice to have a description of those to make the code more accessible to a new contributor/developer

Line 146, about (xramp, yramp) definition: there are a lot attributes, probably tedious to fill in all the info, but it would be cool to have a short description of what are those and why we are computing them.

Line 156, about the call to raised_cosine: np.ndarray doesn't match signature of raised_cosine, which wants Tuple[float, float]
Suggested change
- self.Xrcos, Yrcos = raised_cosine(twidth, (-twidth / 2.0), np.array([0, 1]))
+ self.Xrcos, Yrcos = raised_cosine(twidth, (-twidth / 2.0), (0.,1.))
I just checked the raised cosine function, here it looks like you are squaring the cosine, which is different from what we did for the the GLM basis; In the GLM basis we did use the same formula that was in the original pillow paper and codebase. This is from https://github.com/pillowlab/raisedCosineBasis/blob/master/makeRaisedCosBasis.m
raisedCosFun = @(x,ctr,dCtr)((cos(max(-pi,min(pi,(x-ctr)*pi/dCtr/2)))+1)/2);'
which is (cos(*) +1)/2, and * is truncated to only one bump.
I am not saying you should change what is done here, but I see it can create confusion

This is related to flatironinstitute/log-stretched-basis#2 I think, showing that the various parametrizations are equivalent or at least explaining the relationship between them.

Line 183, in __init__ about self.scales: you'll define a self.scales in the portilla_simoncelli.py too, but the two definition seems slightly different, since self.pyr is an attribute of the PortillaSimoncelli class, is it worth having the scales defined only here, in the steerable pyramid class?

As I commented in #225, they're different because self.scales is model-specific, and the Portilla-Simoncelli texture model additionally considers marginal pixel statistics (first several moments). However, self.scales is only used for coarse-to-fine optimization, and the pyramid generally should not be used as a model by itself (which is why it's in the canonical_computations/ directory instead of models/). So should the pyramid's scales attribute be removed?

Line 196, in __init__ about const: can you compute this one outside the loop?

Line 212, in __init__ about Ycosn_forward+Ycosn_recon: you are using const, self.Xcosn, slef.alpha, self.order, which looks all independent from "i" so you may move this outside the loop too.

general feeling, there are a lot of variables with similar names, Xrcos, Xcosn, is there a way to find names that are more intuitive?